From 33f86a7af15f9752f963d11406350c0e18068a07 Mon Sep 17 00:00:00 2001 From: ichuang Date: Mon, 27 Jan 2014 02:52:48 +0000 Subject: [PATCH 1/5] skip *~ files in import of extra content (e.g. info and about pages) --- common/lib/xmodule/xmodule/modulestore/xml.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 0cdaf7f7e3..98b4ddba93 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -568,6 +568,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') From 09bdcbbc6f02c3f4e5c1f28d2cb956bce0dfceff Mon Sep 17 00:00:00 2001 From: ichuang Date: Mon, 27 Jan 2014 02:42:35 +0000 Subject: [PATCH 2/5] skip ~ files in static content import --- common/lib/xmodule/xmodule/modulestore/xml_importer.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 47dedecc6b..03be931d2b 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/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) From cc091f833a0f3c18ad287548d34946b8cf653b2b Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Thu, 6 Feb 2014 16:41:21 -0500 Subject: [PATCH 3/5] Add test for ignoring tilde files on import of extra content --- .../xmodule/modulestore/tests/test_xml.py | 33 +++++++++++++++++-- common/test/data/tilde/about/index.html | 1 + common/test/data/tilde/about/index.html~ | 1 + common/test/data/tilde/course.xml | 1 + common/test/data/tilde/course/2012_Fall.xml | 2 ++ 5 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 common/test/data/tilde/about/index.html create mode 100644 common/test/data/tilde/about/index.html~ create mode 100644 common/test/data/tilde/course.xml create mode 100644 common/test/data/tilde/course/2012_Fall.xml diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py index 9e15440f11..bdde8b8e6c 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py @@ -1,16 +1,29 @@ 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 +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(object): + +class TestXMLModuleStore(unittest.TestCase): def test_path_to_location(self): """Make sure that path_to_location works properly""" @@ -42,3 +55,19 @@ 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) + diff --git a/common/test/data/tilde/about/index.html b/common/test/data/tilde/about/index.html new file mode 100644 index 0000000000..d991f425fb --- /dev/null +++ b/common/test/data/tilde/about/index.html @@ -0,0 +1 @@ +GREEN diff --git a/common/test/data/tilde/about/index.html~ b/common/test/data/tilde/about/index.html~ new file mode 100644 index 0000000000..f24fc29aa6 --- /dev/null +++ b/common/test/data/tilde/about/index.html~ @@ -0,0 +1 @@ +RED diff --git a/common/test/data/tilde/course.xml b/common/test/data/tilde/course.xml new file mode 100644 index 0000000000..0489d7eed4 --- /dev/null +++ b/common/test/data/tilde/course.xml @@ -0,0 +1 @@ + diff --git a/common/test/data/tilde/course/2012_Fall.xml b/common/test/data/tilde/course/2012_Fall.xml new file mode 100644 index 0000000000..c9d2e8702d --- /dev/null +++ b/common/test/data/tilde/course/2012_Fall.xml @@ -0,0 +1,2 @@ + + From 83b0eda4a47bac74f5fa93c7c83e70ca0abfd0c1 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Fri, 7 Feb 2014 10:23:33 -0500 Subject: [PATCH 4/5] Add test for ignoring tilde static files on import --- .../xmodule/tests/test_import_static.py | 20 +++++++++++++++++++ common/test/data/tilde/static/example.txt | 1 + common/test/data/tilde/static/example.txt~ | 1 + 3 files changed, 22 insertions(+) create mode 100644 common/lib/xmodule/xmodule/tests/test_import_static.py create mode 100644 common/test/data/tilde/static/example.txt create mode 100644 common/test/data/tilde/static/example.txt~ diff --git a/common/lib/xmodule/xmodule/tests/test_import_static.py b/common/lib/xmodule/xmodule/tests/test_import_static.py new file mode 100644 index 0000000000..2a667b05b7 --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_import_static.py @@ -0,0 +1,20 @@ +import unittest +from path import path +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): + 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"]) diff --git a/common/test/data/tilde/static/example.txt b/common/test/data/tilde/static/example.txt new file mode 100644 index 0000000000..d991f425fb --- /dev/null +++ b/common/test/data/tilde/static/example.txt @@ -0,0 +1 @@ +GREEN diff --git a/common/test/data/tilde/static/example.txt~ b/common/test/data/tilde/static/example.txt~ new file mode 100644 index 0000000000..f24fc29aa6 --- /dev/null +++ b/common/test/data/tilde/static/example.txt~ @@ -0,0 +1 @@ +RED From d1dc8c91916b7e5450a5bdf4c6e9ce4318480690 Mon Sep 17 00:00:00 2001 From: David Baumgold Date: Fri, 7 Feb 2014 13:47:16 -0500 Subject: [PATCH 5/5] Add README to tilde fixture, fix pylint/pep8 issues --- .../lib/xmodule/xmodule/modulestore/tests/test_xml.py | 11 +++++++++-- common/lib/xmodule/xmodule/modulestore/xml.py | 7 ++++--- .../lib/xmodule/xmodule/tests/test_import_static.py | 7 +++++-- common/test/data/tilde/README.md | 9 +++++++++ 4 files changed, 27 insertions(+), 7 deletions(-) create mode 100644 common/test/data/tilde/README.md diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py index bdde8b8e6c..71274aae5f 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py @@ -1,3 +1,7 @@ +""" +Tests around our XML modulestore, including importing +well-formed and not-well-formed XML. +""" import os.path import unittest from glob import glob @@ -12,6 +16,7 @@ from xmodule.modulestore import Location, XML_MODULESTORE_TYPE from .test_modulestore import check_path_to_location from xmodule.tests import DATA_DIR + def glob_tildes_at_end(path): """ A wrapper for the `glob.glob` function, but it always returns @@ -24,6 +29,9 @@ def glob_tildes_at_end(path): class TestXMLModuleStore(unittest.TestCase): + """ + Test around the XML modulestore + """ def test_path_to_location(self): """Make sure that path_to_location works properly""" @@ -57,7 +65,7 @@ class TestXMLModuleStore(unittest.TestCase): assert errors == [] @patch("xmodule.modulestore.xml.glob.glob", side_effect=glob_tildes_at_end) - def test_tilde_files_ignored(self, fake_glob): + 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({ @@ -70,4 +78,3 @@ class TestXMLModuleStore(unittest.TestCase): about_module = course_module[about_location] self.assertIn("GREEN", about_module.data) self.assertNotIn("RED", about_module.data) - diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 98b4ddba93..5b55fa1807 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/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,7 +569,7 @@ class XMLModuleStore(ModuleStoreReadBase): if not os.path.isfile(filepath): continue - if filepath.endswith('~'): # skip *~ files + if filepath.endswith('~'): # skip *~ files continue with open(filepath) as f: diff --git a/common/lib/xmodule/xmodule/tests/test_import_static.py b/common/lib/xmodule/xmodule/tests/test_import_static.py index 2a667b05b7..f19d41aaf4 100644 --- a/common/lib/xmodule/xmodule/tests/test_import_static.py +++ b/common/lib/xmodule/xmodule/tests/test_import_static.py @@ -1,5 +1,7 @@ +""" +Tests that check that we ignore the appropriate files when importing courses. +""" import unittest -from path import path from mock import Mock from xmodule.modulestore import Location from xmodule.modulestore.xml_importer import import_static_content @@ -7,8 +9,9 @@ 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" + course_dir = DATA_DIR / "tilde" loc = Location("edX", "tilde", "Fall_2012") content_store = Mock() content_store.generate_thumbnail.return_value = ("content", "location") diff --git a/common/test/data/tilde/README.md b/common/test/data/tilde/README.md new file mode 100644 index 0000000000..7f24e3e75e --- /dev/null +++ b/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.