diff --git a/cms/djangoapps/contentstore/views/import_export.py b/cms/djangoapps/contentstore/views/import_export.py index 8d7f1aac768f..df6d960bba36 100644 --- a/cms/djangoapps/contentstore/views/import_export.py +++ b/cms/djangoapps/contentstore/views/import_export.py @@ -154,7 +154,7 @@ def wfile(filename, dirname): sf.write("Extracting") tar_file = tarfile.open(temp_filepath) - tar_file.extractall(course_dir + '/') + tar_file.extractall((course_dir + '/').encode('utf-8')) with open(status_file, 'w+') as sf: sf.write("Verifying") diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 6f3276bf446a..ba1266cb146b 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -2,8 +2,10 @@ # pylint: disable=E0611 from nose.tools import assert_equals, assert_raises, \ assert_not_equals, assert_false +from itertools import ifilter # pylint: enable=E0611 import pymongo +import logging from uuid import uuid4 from xblock.fields import Scope @@ -19,6 +21,7 @@ from xmodule.modulestore.tests.test_modulestore import check_path_to_location +log = logging.getLogger(__name__) HOST = 'localhost' PORT = 27017 @@ -59,7 +62,7 @@ def initdb(): # draft_store = DraftModuleStore(HOST, DB, COLLECTION, FS_ROOT, RENDER_TEMPLATE, default_class=DEFAULT_CLASS) # Explicitly list the courses to load (don't want the big one) - courses = ['toy', 'simple', 'simple_with_draft'] + courses = ['toy', 'simple', 'simple_with_draft', 'test_unicode'] import_from_xml(store, DATA_DIR, courses, draft_store=draft_store, static_content_store=content_store) # also test a course with no importing of static content @@ -86,6 +89,19 @@ def setUp(self): def tearDown(self): pass + def get_course_by_id(self, name): + """ + Returns the first course with `id` of `name`, or `None` if there are none. + """ + courses = self.store.get_courses() + return next(ifilter(lambda x: x.id == name, courses), None) + + def course_with_id_exists(self, name): + """ + Returns true iff there exists some course with `id` of `name`. + """ + return (self.get_course_by_id(name) is not None) + def test_init(self): '''Make sure the db loads, and print all the locations in the db. Call this directly from failing tests to see what is loaded''' @@ -100,12 +116,12 @@ def test_mongo_modulestore_type(self): def test_get_courses(self): '''Make sure the course objects loaded properly''' courses = self.store.get_courses() - assert_equals(len(courses), 4) - courses.sort(key=lambda c: c.id) - assert_equals(courses[0].id, 'edX/simple/2012_Fall') - assert_equals(courses[1].id, 'edX/simple_with_draft/2012_Fall') - assert_equals(courses[2].id, 'edX/test_import_course/2012_Fall') - assert_equals(courses[3].id, 'edX/toy/2012_Fall') + assert_equals(len(courses), 5) + assert self.course_with_id_exists('edX/simple/2012_Fall') + assert self.course_with_id_exists('edX/simple_with_draft/2012_Fall') + assert self.course_with_id_exists('edX/test_import_course/2012_Fall') + assert self.course_with_id_exists('edX/test_unicode/2012_Fall') + assert self.course_with_id_exists('edX/toy/2012_Fall') def test_loads(self): assert_not_equals( @@ -120,6 +136,22 @@ def test_loads(self): self.store.get_item("i4x://edX/toy/video/Welcome"), None) + def test_unicode_loads(self): + assert_not_equals( + self.store.get_item("i4x://edX/test_unicode/course/2012_Fall"), + None) + # All items with ascii-only filenames should load properly. + assert_not_equals( + self.store.get_item("i4x://edX/test_unicode/video/Welcome"), + None) + assert_not_equals( + self.store.get_item("i4x://edX/test_unicode/video/Welcome"), + None) + assert_not_equals( + self.store.get_item("i4x://edX/test_unicode/chapter/Overview"), + None) + + def test_find_one(self): assert_not_equals( self.store._find_one(Location("i4x://edX/toy/course/2012_Fall")), @@ -153,15 +185,15 @@ def test_get_courses_has_no_templates(self): ) def test_static_tab_names(self): - courses = self.store.get_courses() def get_tab_name(index): """ Helper function for pulling out the name of a given static tab. - Assumes the information is desired for courses[1] ('toy' course). + Assumes the information is desired for courses[4] ('toy' course). """ - return courses[2].tabs[index]['name'] + course = self.get_course_by_id('edX/toy/2012_Fall') + return course.tabs[index]['name'] # There was a bug where model.save was not getting called after the static tab name # was set set for tabs that have a URL slug. 'Syllabus' and 'Resources' fall into that diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index dd7888179b3c..62fd6c30c975 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -173,7 +173,7 @@ def fallback_name(orig_name=None): # Didn't load properly. Fall back on loading as an error # descriptor. This should never error due to formatting. - msg = "Error loading from xml. " + str(err)[:200] + msg = "Error loading from xml. " + unicode(err)[:200] log.warning(msg) # Normally, we don't want lots of exception traces in our logs from common # content problems. But if you're debugging the xml loading code itself, @@ -317,7 +317,8 @@ def try_load_course(self, course_dir): 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, str(e)) + msg = "ERROR: Failed to load course '{0}': {1}".format(course_dir.encode("utf-8"), + unicode(e)) log.exception(msg) errorlog.tracker(msg) @@ -493,8 +494,9 @@ def _load_extra_content(self, system, course_descriptor, category, path, course_ module.save() self.modules[course_descriptor.id][module.location] = module except Exception, e: - logging.exception("Failed to load {0}. Skipping... Exception: {1}".format(filepath, str(e))) - system.error_tracker("ERROR: " + str(e)) + logging.exception("Failed to load %s. Skipping... \ + Exception: %s", filepath, unicode(e)) + system.error_tracker("ERROR: " + unicode(e)) def get_instance(self, course_id, location, depth=0): """ diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index a9a7601e334f..4a4b25774d94 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -31,7 +31,7 @@ def import_static_content(modules, course_loc, course_data_path, static_content_ try: content_path = os.path.join(dirname, filename) if verbose: - log.debug('importing static content {0}...'.format(content_path)) + log.debug('importing static content %s...', content_path) fullname_with_subpath = content_path.replace(static_dir, '') # strip away leading path from the name if fullname_with_subpath.startswith('/'): diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index ebd49d75cf74..867eae19cecc 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -133,7 +133,7 @@ def definition_from_xml(cls, xml_object, system): except Exception as e: log.exception("Unable to load child when parsing Sequence. Continuing...") if system.error_tracker is not None: - system.error_tracker("ERROR: " + str(e)) + system.error_tracker("ERROR: " + unicode(e)) continue return {}, children diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 8f24112bc6e3..19232e383fce 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -368,6 +368,32 @@ def test_colon_in_url_name(self): html = modulestore.get_instance(course_id, loc) self.assertEquals(html.display_name, "Toy lab") + def test_unicode(self): + """Check that courses with unicode characters in filenames and in + org/course/name import properly. Currently, this means: (a) Having + files with unicode names does not prevent import; (b) if files are not + loaded because of unicode filenames, there are appropriate + exceptions/errors to that effect.""" + + print("Starting import") + modulestore = XMLModuleStore(DATA_DIR, course_dirs=['test_unicode']) + courses = modulestore.get_courses() + self.assertEquals(len(courses), 1) + course = courses[0] + + print("course errors:") + + # Expect to find an error/exception about characters in "®esources" + expect = "Invalid characters in '®esources'" + errors = [(msg.encode("utf-8"), err.encode("utf-8")) + for msg, err in + modulestore.get_item_errors(course.location)] + + self.assertTrue(any(expect in msg or expect in err + for msg, err in errors)) + chapters = course.get_children() + self.assertEqual(len(chapters), 3) + def test_url_name_mangling(self): """ Make sure that url_names are only mangled once. diff --git a/common/test/data/test_unicode/about/end_date.html b/common/test/data/test_unicode/about/end_date.html new file mode 100644 index 000000000000..a0990367ef8b --- /dev/null +++ b/common/test/data/test_unicode/about/end_date.html @@ -0,0 +1 @@ +TBD diff --git a/common/test/data/test_unicode/chapter/simple_html.xml b/common/test/data/test_unicode/chapter/simple_html.xml new file mode 100644 index 000000000000..c07cf4a7c766 --- /dev/null +++ b/common/test/data/test_unicode/chapter/simple_html.xml @@ -0,0 +1,8 @@ + + + +

This is upside down text:

+

˙ʇxəʇ uʍop əpısdn sı sıɥʇ

+ +
+
diff --git a/common/test/data/test_unicode/course.xml b/common/test/data/test_unicode/course.xml new file mode 100644 index 000000000000..0375e11d3ac2 --- /dev/null +++ b/common/test/data/test_unicode/course.xml @@ -0,0 +1 @@ + diff --git a/common/test/data/test_unicode/course/2012_Fall.xml b/common/test/data/test_unicode/course/2012_Fall.xml new file mode 100644 index 000000000000..b402922bcb57 --- /dev/null +++ b/common/test/data/test_unicode/course/2012_Fall.xml @@ -0,0 +1,8 @@ + + + + + + + diff --git a/common/test/data/test_unicode/policies/2012_Fall.json b/common/test/data/test_unicode/policies/2012_Fall.json new file mode 100644 index 000000000000..71f88849ec79 --- /dev/null +++ b/common/test/data/test_unicode/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": "Üñîçø∂e †es† Course", + "graded": "true", + "tabs": [ + {"type": "courseware"}, + {"type": "course_info", "name": "Course Info"}, + {"type": "static_tab", "url_slug": "syllabus", "name": "ßyllabus"}, + {"type": "static_tab", "url_slug": "resources", "name": "®esources"}, + {"type": "discussion", "name": "∂iscussion"}, + {"type": "wiki", "name": "∑iki"}, + {"type": "progress", "name": "πrogress"} + ] + }, + "chapter/Overview": { + "display_name": "O√erview" + }, + "video/Welcome": { + "display_name": "Welcome" + } +} diff --git a/common/test/data/test_unicode/sequential/vertical_sequential.xml b/common/test/data/test_unicode/sequential/vertical_sequential.xml new file mode 100644 index 000000000000..695e640243d5 --- /dev/null +++ b/common/test/data/test_unicode/sequential/vertical_sequential.xml @@ -0,0 +1,4 @@ + + + … + \ No newline at end of file diff --git "a/common/test/data/test_unicode/static/unic\303\270\342\210\202e\314\201.txt" "b/common/test/data/test_unicode/static/unic\303\270\342\210\202e\314\201.txt" new file mode 100644 index 000000000000..44d66a740044 --- /dev/null +++ "b/common/test/data/test_unicode/static/unic\303\270\342\210\202e\314\201.txt" @@ -0,0 +1,10 @@ + +Upside down unicode text (http://www.sunnyneo.com/upsidedowntext.php?) + +˙ʇʇɐld uoɯəl plos əɥs ˙pəʌıl əuɹʎq ʎʇʇəq əɹəɥʍ pɐoɹ əɥʇ uʍop əɯɐɔ ʍoɔ-ooɯ əɥʇ +˙ooʞɔnʇ ʎqɐq sɐʍ əɥ ˙əɔɐɟ ʎɹıɐɥ ɐ pɐɥ əɥ :ssɐlƃ ɐ ɥƃnoɹɥʇ ɯıɥ ʇɐ pəʞool ɹəɥʇɐɟ +sıɥ :ʎɹoʇs ʇɐɥʇ ɯıɥ ploʇ ɹəɥʇɐɟ sıɥ + +˙˙˙ooʞɔnʇ ʎqɐq pəɯɐu ʎoq əlʇʇıl suəɔıu ɐ ʇəɯ pɐoɹ əɥʇ ƃuolɐ uʍop ƃuıɯoɔ sɐʍ ʇɐɥʇ +ʍoɔ-ooɯ sıɥʇ puɐ pɐoɹ əɥʇ ƃuolɐ uʍop ƃuıɯoɔ ʍoɔ-ooɯ ɐ sɐʍ əɹəɥʇ sɐʍ ʇı əɯıʇ pooƃ +ʎɹəʌ ɐ puɐ əɯıʇ ɐ uodn əɔuo diff --git "a/common/test/data/test_unicode/tabs/\302\256esources.html" "b/common/test/data/test_unicode/tabs/\302\256esources.html" new file mode 100644 index 000000000000..0f6bdf0984ba --- /dev/null +++ "b/common/test/data/test_unicode/tabs/\302\256esources.html" @@ -0,0 +1 @@ +
resources!
diff --git a/common/test/data/test_unicode/vertical/vertical_test.xml b/common/test/data/test_unicode/vertical/vertical_test.xml new file mode 100644 index 000000000000..82df156dfe18 --- /dev/null +++ b/common/test/data/test_unicode/vertical/vertical_test.xml @@ -0,0 +1,3 @@ + + diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 372aa91f8418..5c8c7d79e180 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -16,5 +16,5 @@ # Our libraries: -e git+https://github.com/edx/XBlock.git@aa0d60627#egg=XBlock -e git+https://github.com/edx/codejail.git@0a1b468#egg=codejail --e git+https://github.com/edx/diff-cover.git@v0.2.2#egg=diff_cover +-e git+https://github.com/edx/diff-cover.git@v0.2.3#egg=diff_cover -e git+https://github.com/edx/js-test-tool.git@v0.0.7#egg=js_test_tool