From 6a24ecebe7e2eadf9b89d2054ad608cbd94144d1 Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Wed, 28 Nov 2012 17:54:18 -0500 Subject: [PATCH 1/9] decode utf-8 when reading html, and encode when writing. --- common/djangoapps/mitxmako/shortcuts.py | 2 +- common/djangoapps/mitxmako/template.py | 2 +- common/lib/xmodule/xmodule/html_module.py | 4 ++-- common/lib/xmodule/xmodule/template_module.py | 2 +- common/lib/xmodule/xmodule/xml_module.py | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/common/djangoapps/mitxmako/shortcuts.py b/common/djangoapps/mitxmako/shortcuts.py index ba22f2db20..181d3befd5 100644 --- a/common/djangoapps/mitxmako/shortcuts.py +++ b/common/djangoapps/mitxmako/shortcuts.py @@ -42,7 +42,7 @@ def render_to_string(template_name, dictionary, context=None, namespace='main'): context_dictionary.update(context) # fetch and render template template = middleware.lookup[namespace].get_template(template_name) - return template.render(**context_dictionary) + return template.render_unicode(**context_dictionary) def render_to_response(template_name, dictionary, context_instance=None, namespace='main', **kwargs): diff --git a/common/djangoapps/mitxmako/template.py b/common/djangoapps/mitxmako/template.py index 56096fe173..2d6fc026ca 100644 --- a/common/djangoapps/mitxmako/template.py +++ b/common/djangoapps/mitxmako/template.py @@ -54,5 +54,5 @@ class Template(MakoTemplate): context_dictionary['MITX_ROOT_URL'] = settings.MITX_ROOT_URL context_dictionary['django_context'] = context_instance - return super(Template, self).render(**context_dictionary) + return super(Template, self).render_unicode(**context_dictionary) diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 2023ac7017..4f10cc84f1 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -123,7 +123,7 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor): try: with system.resources_fs.open(filepath) as file: - html = file.read() + html = file.read().decode('utf-8') # Log a warning if we can't parse the file, but don't error if not check_html(html): msg = "Couldn't parse html in {0}.".format(filepath) @@ -164,7 +164,7 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor): resource_fs.makedir(os.path.dirname(filepath), allow_recreate=True) with resource_fs.open(filepath, 'w') as file: - file.write(self.definition['data']) + file.write(self.definition['data'].encode('utf-8')) # write out the relative name relname = path(pathname).basename() diff --git a/common/lib/xmodule/xmodule/template_module.py b/common/lib/xmodule/xmodule/template_module.py index 13eab038ec..d3fb0aab5e 100644 --- a/common/lib/xmodule/xmodule/template_module.py +++ b/common/lib/xmodule/xmodule/template_module.py @@ -58,7 +58,7 @@ class CustomTagDescriptor(RawDescriptor): params = dict(xmltree.items()) with system.resources_fs.open('custom_tags/{name}' .format(name=template_name)) as template: - return Template(template.read()).render(**params) + return Template(template.read().decode('utf-8')).render(**params) def __init__(self, system, definition, **kwargs): diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index ec755af4ef..e65a8c74ea 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -366,7 +366,7 @@ class XmlDescriptor(XModuleDescriptor): filepath = self.__class__._format_filepath(self.category, url_path) resource_fs.makedir(os.path.dirname(filepath), allow_recreate=True) with resource_fs.open(filepath, 'w') as file: - file.write(etree.tostring(xml_object, pretty_print=True)) + file.write(etree.tostring(xml_object, pretty_print=True, encoding='utf-8', xml_declaration=True)) # And return just a pointer with the category and filename. record_object = etree.Element(self.category) From 9bfa800208de908792e424cf21ff4adfb06b7707 Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Wed, 28 Nov 2012 18:28:40 -0500 Subject: [PATCH 2/9] add non-ascii data to toy dataset --- common/test/data/toy/html/secret/toylab.html | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/common/test/data/toy/html/secret/toylab.html b/common/test/data/toy/html/secret/toylab.html index 81df84bd63..760482c4a0 100644 --- a/common/test/data/toy/html/secret/toylab.html +++ b/common/test/data/toy/html/secret/toylab.html @@ -1,3 +1,9 @@ Lab 2A: Superposition Experiment

Isn't the toy course great?

+ +

Let's add some markup that uses non-ascii characters. +For example, we should be able to write words like encyclopædia, or foreign words like français. +Looking beyond latin-1, we should handle math symbols: πr² ≤ ∞. +And it shouldn't matter if we use entities or numeric codes — Ω ≠ π ≡ Ω ≠ π. +

From 0c1ebd8dfb3a10f0b6321d72c38adca02de28e1a Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Fri, 30 Nov 2012 19:05:26 -0500 Subject: [PATCH 3/9] add toylab test data and test changes, including fix to jump_to --- common/lib/xmodule/xmodule/html_module.py | 4 +- common/test/data/toy/html/secret/toylab.html | 4 + lms/djangoapps/courseware/tests/tests.py | 101 +++++++++++++++---- lms/djangoapps/courseware/views.py | 17 ++-- 4 files changed, 101 insertions(+), 25 deletions(-) diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 4f10cc84f1..eea747e332 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -30,7 +30,9 @@ class HtmlModule(XModule): def get_html(self): # cdodge: perform link substitutions for any references to course static content (e.g. images) - return rewrite_links(self.html, self.rewrite_content_links) + output = rewrite_links(self.html, self.rewrite_content_links) + log.info(' HTMLModule converting markup "{0}" to "{1}"'.format(self.html, output)) + return output def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): diff --git a/common/test/data/toy/html/secret/toylab.html b/common/test/data/toy/html/secret/toylab.html index 760482c4a0..b2a4599cc6 100644 --- a/common/test/data/toy/html/secret/toylab.html +++ b/common/test/data/toy/html/secret/toylab.html @@ -1,5 +1,6 @@ Lab 2A: Superposition Experiment +<<<<<<< Updated upstream

Isn't the toy course great?

Let's add some markup that uses non-ascii characters. @@ -7,3 +8,6 @@ For example, we should be able to write words like encyclopædia, or foreig Looking beyond latin-1, we should handle math symbols: πr² ≤ ∞. And it shouldn't matter if we use entities or numeric codes — Ω ≠ π ≡ Ω ≠ π.

+======= +

Isn't the toy course great? — ≤

+>>>>>>> Stashed changes diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 8239eadfd9..defbf426cc 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -1,21 +1,15 @@ -import copy import json import os -import sys import time from nose import SkipTest -from path import path -from pprint import pprint from urlparse import urlsplit, urlunsplit from django.contrib.auth.models import User, Group -from django.core.handlers.wsgi import WSGIRequest from django.test import TestCase -from django.test.client import Client, RequestFactory +from django.test.client import RequestFactory from django.conf import settings from django.core.urlresolvers import reverse -from mock import patch, Mock from override_settings import override_settings import xmodule.modulestore.django @@ -29,6 +23,7 @@ from student.models import Registration from xmodule.modulestore.django import modulestore from xmodule.modulestore import Location from xmodule.modulestore.xml_importer import import_from_xml +from xmodule.modulestore.xml import XMLModuleStore from xmodule.timeparse import stringify_time def parse_json(response): @@ -76,10 +71,21 @@ def xml_store_config(data_dir): } } +def my_xml_store_config(data_dir): + return { + 'default': { + 'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore', + 'OPTIONS': { + 'data_dir': data_dir, + 'default_class': 'xmodule.hidden_module.HiddenDescriptor', + } + } +} TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT -TEST_DATA_MONGO_MODULESTORE = mongo_store_config(TEST_DATA_DIR) +# TEST_DATA_MONGO_MODULESTORE = mongo_store_config(TEST_DATA_DIR) TEST_DATA_XML_MODULESTORE = xml_store_config(TEST_DATA_DIR) +MY_TEST_DATA_XML_MODULESTORE = my_xml_store_config(TEST_DATA_DIR) REAL_DATA_DIR = settings.GITHUB_REPO_ROOT REAL_DATA_MODULESTORE = mongo_store_config(REAL_DATA_DIR) @@ -252,34 +258,93 @@ class PageLoader(ActivateLoginTestCase): #print descriptor.__class__, descriptor.location resp = self.client.get(reverse('jump_to', kwargs={'course_id': course_id, - 'location': descriptor.location.url()})) + 'location': descriptor.location.url()}), follow=True) msg = str(resp.status_code) - - if resp.status_code != 302: + if resp.status_code != 200: + msg = "ERROR " + msg # + ": " + str(resp.request['PATH_INFO']) + all_ok = False + num_bad += 1 + elif resp.redirect_chain[0][1] != 302: msg = "ERROR " + msg all_ok = False num_bad += 1 print msg - self.assertTrue(all_ok) # fail fast +# self.assertTrue(all_ok) # fail fast + + print "{0}/{1} good".format(n - num_bad, n) + self.assertTrue(all_ok) + + def check_xml_pages_load(self, course_name, data_dir, modstore): + """Make all locations in course load""" + print "Checking course {0} in {1}".format(course_name, data_dir) + default_class='xmodule.hidden_module.HiddenDescriptor' # 'xmodule.raw_module.RawDescriptor', + load_error_modules=True + module_store = XMLModuleStore( + data_dir, + default_class=default_class, + course_dirs=[course_name], + load_error_modules=load_error_modules, + ) +# for course_id in module_store.modules.keys(): +# for module in module_store.modules[course_id].itervalues(): +# +# if 'data' in module.definition: +# store.update_item(module.location, module.definition['data']) +# if 'children' in module.definition: +# store.update_children(module.location, module.definition['children']) +# # NOTE: It's important to use own_metadata here to avoid writing +# # inherited metadata everywhere. +# store.update_metadata(module.location, dict(module.own_metadata)) + # enroll in the course before trying to access pages + courses = module_store.get_courses() + self.assertEqual(len(courses), 1) + course = courses[0] + self.enroll(course) + course_id = course.id + + n = 0 + num_bad = 0 + all_ok = True + for descriptor in module_store.modules[course_id].itervalues(): + n += 1 + print "Checking ", descriptor.location.url() + #print descriptor.__class__, descriptor.location + resp = self.client.get(reverse('jump_to', + kwargs={'course_id': course_id, + 'location': descriptor.location.url()}), follow=True) + msg = str(resp.status_code) + if resp.status_code != 200: + msg = "ERROR " + msg # + ": " + str(resp.request['PATH_INFO']) + all_ok = False + num_bad += 1 + elif resp.redirect_chain[0][1] != 302: + msg = "ERROR " + msg + all_ok = False + num_bad += 1 + print msg +# self.assertTrue(all_ok) # fail fast print "{0}/{1} good".format(n - num_bad, n) self.assertTrue(all_ok) -@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) +#@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) +@override_settings(MODULESTORE=MY_TEST_DATA_XML_MODULESTORE) class TestCoursesLoadTestCase(PageLoader): '''Check that all pages in test courses load properly''' def setUp(self): ActivateLoginTestCase.setUp(self) xmodule.modulestore.django._MODULESTORES = {} - xmodule.modulestore.django.modulestore().collection.drop() - +# xmodule.modulestore.django.modulestore().collection.drop() +# store = xmodule.modulestore.django.modulestore() + # is there a way to empty the store? + def test_toy_course_loads(self): - self.check_pages_load('toy', TEST_DATA_DIR, modulestore()) + self.check_xml_pages_load('toy', TEST_DATA_DIR, modulestore()) - def test_full_course_loads(self): - self.check_pages_load('full', TEST_DATA_DIR, modulestore()) +# def test_full_course_loads(self): +# self.check_pages_load('full', TEST_DATA_DIR, modulestore()) @override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 73d40b05c5..276af80ca9 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -293,7 +293,6 @@ def index(request, course_id, chapter=None, section=None, return result - @ensure_csrf_cookie def jump_to(request, course_id, location): ''' @@ -318,12 +317,18 @@ def jump_to(request, course_id, location): except NoPathToItem: raise Http404("This location is not in any class: {0}".format(location)) + # choose the appropriate view (and provide the necessary args) based on the + # args provided by the redirect. # Rely on index to do all error handling and access control. - return redirect('courseware_position', - course_id=course_id, - chapter=chapter, - section=section, - position=position) + if chapter is None: + return redirect('courseware', course_id=course_id) + elif section is None: + return redirect('courseware_chapter', course_id=course_id, chapter=chapter) + elif position is None: + return redirect('courseware_section', course_id=course_id, chapter=chapter, section=section) + else: + return redirect('courseware_position', course_id=course_id, chapter=chapter, section=section, position=position) + @ensure_csrf_cookie def course_info(request, course_id): """ From 1d44ebb10c842b02371dd92af02db671785ce0b0 Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Mon, 3 Dec 2012 16:22:25 -0500 Subject: [PATCH 4/9] Add more non-ascii characters to full dataset, and make fixes in xmodule code. --- common/djangoapps/mitxmako/shortcuts.py | 7 ++++- common/djangoapps/mitxmako/template.py | 11 +++++-- common/lib/xmodule/xmodule/html_module.py | 6 ++-- common/test/data/full/custom_tags/book | 2 +- common/test/data/full/custom_tags/discuss | 2 +- common/test/data/full/custom_tags/slides | 2 +- .../data/full/problem/Circuit_Sandbox.xml | 4 +-- .../data/full/problem/choiceresponse_demo.xml | 15 ++++----- .../test/data/full/problem/codeinput_demo.xml | 3 +- .../Administrivia_and_Circuit_Elements.xml | 4 +-- .../test/data/full/vertical/vertical_89.xml | 2 +- common/test/data/full/video/welcome.xml | 2 +- lms/djangoapps/courseware/tests/tests.py | 31 ++++++++++++++++--- 13 files changed, 64 insertions(+), 27 deletions(-) diff --git a/common/djangoapps/mitxmako/shortcuts.py b/common/djangoapps/mitxmako/shortcuts.py index 181d3befd5..6aee39906a 100644 --- a/common/djangoapps/mitxmako/shortcuts.py +++ b/common/djangoapps/mitxmako/shortcuts.py @@ -42,7 +42,12 @@ def render_to_string(template_name, dictionary, context=None, namespace='main'): context_dictionary.update(context) # fetch and render template template = middleware.lookup[namespace].get_template(template_name) - return template.render_unicode(**context_dictionary) +# return template.render_unicode(**context_dictionary) + + output = template.render_unicode(**context_dictionary) +# log.info(' render_to_string of "{0}" as "{1}r"'.format(type(output), output)) + return output +# return template.render(**context_dictionary) def render_to_response(template_name, dictionary, context_instance=None, namespace='main', **kwargs): diff --git a/common/djangoapps/mitxmako/template.py b/common/djangoapps/mitxmako/template.py index 2d6fc026ca..efeb282d04 100644 --- a/common/djangoapps/mitxmako/template.py +++ b/common/djangoapps/mitxmako/template.py @@ -12,6 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. +import logging +log = logging.getLogger("mitx." + __name__) + from django.conf import settings from mako.template import Template as MakoTemplate @@ -54,5 +57,9 @@ class Template(MakoTemplate): context_dictionary['MITX_ROOT_URL'] = settings.MITX_ROOT_URL context_dictionary['django_context'] = context_instance - return super(Template, self).render_unicode(**context_dictionary) - +# return super(Template, self).render_unicode(**context_dictionary) +# return super(Template, self).render(**context_dictionary) + + output = super(Template, self).render(**context_dictionary) + log.info(' render_to_string of "{0}" as "{1}"'.format(type(output), output)) + return output diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index eea747e332..6b73535dac 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -30,8 +30,10 @@ class HtmlModule(XModule): def get_html(self): # cdodge: perform link substitutions for any references to course static content (e.g. images) + input = self.html output = rewrite_links(self.html, self.rewrite_content_links) - log.info(' HTMLModule converting markup "{0}" to "{1}"'.format(self.html, output)) +# log.info(' HTMLModule converting markup from "{0}" as "{1}r"'.format(type(input), input)) +# log.info(' HTMLModule converting markup to "{0}" as "{1}r"'.format(type(output), output)) return output def __init__(self, system, location, definition, descriptor, @@ -166,7 +168,7 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor): resource_fs.makedir(os.path.dirname(filepath), allow_recreate=True) with resource_fs.open(filepath, 'w') as file: - file.write(self.definition['data'].encode('utf-8')) + file.write(self.definition['data']) # .encode('utf-8')) # write out the relative name relname = path(pathname).basename() diff --git a/common/test/data/full/custom_tags/book b/common/test/data/full/custom_tags/book index ece6f288db..32e0913e6d 100644 --- a/common/test/data/full/custom_tags/book +++ b/common/test/data/full/custom_tags/book @@ -1 +1 @@ -More information given in the text. +More information given in… the text. diff --git a/common/test/data/full/custom_tags/discuss b/common/test/data/full/custom_tags/discuss index ac56590074..7a8a9e985f 100644 --- a/common/test/data/full/custom_tags/discuss +++ b/common/test/data/full/custom_tags/discuss @@ -1 +1 @@ - Discussion: ${tag} \ No newline at end of file + Discussion: ${tag}… \ No newline at end of file diff --git a/common/test/data/full/custom_tags/slides b/common/test/data/full/custom_tags/slides index a93d94947c..967c203711 100644 --- a/common/test/data/full/custom_tags/slides +++ b/common/test/data/full/custom_tags/slides @@ -1 +1 @@ -Lecture Slides Handout [Clean ][Annotated] +Lecture Slides Handout [Clean… ][Annotated…] diff --git a/common/test/data/full/problem/Circuit_Sandbox.xml b/common/test/data/full/problem/Circuit_Sandbox.xml index 89625f447b..1582f3ff0b 100644 --- a/common/test/data/full/problem/Circuit_Sandbox.xml +++ b/common/test/data/full/problem/Circuit_Sandbox.xml @@ -1,6 +1,6 @@ -

Here's a sandbox where you can experiment with all the components +

Here's a sandbox where you can experiment with all the components we'll discuss in 6.002x. If you click on CHECK below, your diagram -will be saved on the server and you can return at some later time. +will be saved on the server and you can return at some later time…

correct = ['correct'] diff --git a/common/test/data/full/problem/choiceresponse_demo.xml b/common/test/data/full/problem/choiceresponse_demo.xml index f7d1fcf16c..7af7939d74 100644 --- a/common/test/data/full/problem/choiceresponse_demo.xml +++ b/common/test/data/full/problem/choiceresponse_demo.xml @@ -1,19 +1,20 @@ -

Consider a hypothetical magnetic field pointing out of your computer screen. Now imagine an electron traveling from right to leftin the plane of your screen. A diagram of this situation is show below.

+

Consider a hypothetical magnetic field pointing out of your computer screen. Now imagine an electron traveling from right to left in the plane of your screen. A diagram of this situation is show below…

a. The magnitude of the force experienced by the electron is proportional the product of which of the following? (Select all that apply.)

-Magnetic field strength -Electric field strength -Electric charge of the electron -Radius of the electron -Mass of the electron -Velocity of the electron + +Magnetic field strength… +Electric field strength… +Electric charge of the electron… +Radius of the electron… +Mass of the electron… +Velocity of the electron… diff --git a/common/test/data/full/problem/codeinput_demo.xml b/common/test/data/full/problem/codeinput_demo.xml index 03d8fd8c31..a6662cb69c 100644 --- a/common/test/data/full/problem/codeinput_demo.xml +++ b/common/test/data/full/problem/codeinput_demo.xml @@ -2,7 +2,8 @@

- Part 1: Function Types + + Part 1: Function Types…

For each of the following functions, specify the type of its output. You can assume each function is called with an appropriate argument, as specified by its docstring.

diff --git a/common/test/data/full/sequential/Administrivia_and_Circuit_Elements.xml b/common/test/data/full/sequential/Administrivia_and_Circuit_Elements.xml index 5c4c65f12d..d0239198af 100644 --- a/common/test/data/full/sequential/Administrivia_and_Circuit_Elements.xml +++ b/common/test/data/full/sequential/Administrivia_and_Circuit_Elements.xml @@ -3,12 +3,12 @@ - S1E4 has been removed. + S1E4 has been removed… diff --git a/common/test/data/full/vertical/vertical_89.xml b/common/test/data/full/vertical/vertical_89.xml index da15a6751a..a4716366fe 100644 --- a/common/test/data/full/vertical/vertical_89.xml +++ b/common/test/data/full/vertical/vertical_89.xml @@ -1,6 +1,6 @@ -

+

Inline content…