From 50bea28d750a8d5bfbe9ff31b5bdef18c2957bdd Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 12 Jul 2013 11:13:59 -0400 Subject: [PATCH 1/6] Add a 'string literal' serialization method and use it in the lambda function mapping table when the value to xml serialize is of basestring type. Also a couple of drive-by pep8 fixes. --- common/lib/xmodule/xmodule/xml_module.py | 54 ++++++++++++++---------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index c1340a9fc0..edd10288de 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -81,13 +81,23 @@ class AttrMap(_AttrMapBase): def serialize_field(value): """ - Return a string version of the value (where value is the JSON-formatted, internally stored value). + Return a string version of the value (where value is the JSON-formatted, internally stored value). - By default, this is the result of calling json.dumps on the input value. - """ + By default, this is the result of calling json.dumps on the input value. + """ return json.dumps(value, cls=EdxJSONEncoder) +def serialize_string_literal(value): + """ + Assert that the value is a base string and - if it is - simply return it + """ + if not isinstance(value, basestring): + raise Exception('Value {0} is not of type basestring!'.format(value)) + + return value + + def deserialize_field(field, value): """ Deserialize the string version to the value stored internally. @@ -126,7 +136,7 @@ class XmlDescriptor(XModuleDescriptor): """ xml_attributes = Dict(help="Map of unhandled xml attributes, used only for storage between import and export", - default={}, scope=Scope.settings) + default={}, scope=Scope.settings) # Extension to append to filename paths filename_extension = 'xml' @@ -141,23 +151,23 @@ class XmlDescriptor(XModuleDescriptor): # understand? And if we do, is this the place? # Related: What's the right behavior for clean_metadata? metadata_attributes = ('format', 'graceperiod', 'showanswer', 'rerandomize', - 'start', 'due', 'graded', 'display_name', 'url_name', 'hide_from_toc', - 'ispublic', # if True, then course is listed for all users; see - 'xqa_key', # for xqaa server access - 'giturl', # url of git server for origin of file - # information about testcenter exams is a dict (of dicts), not a string, - # so it cannot be easily exportable as a course element's attribute. - 'testcenter_info', - # VS[compat] Remove once unused. - 'name', 'slug') + 'start', 'due', 'graded', 'display_name', 'url_name', 'hide_from_toc', + 'ispublic', # if True, then course is listed for all users; see + 'xqa_key', # for xqaa server access + 'giturl', # url of git server for origin of file + # information about testcenter exams is a dict (of dicts), not a string, + # so it cannot be easily exportable as a course element's attribute. + 'testcenter_info', + # VS[compat] Remove once unused. + 'name', 'slug') metadata_to_strip = ('data_dir', - 'tabs', 'grading_policy', 'published_by', 'published_date', - 'discussion_blackouts', 'testcenter_info', - # VS[compat] -- remove the below attrs once everything is in the CMS - 'course', 'org', 'url_name', 'filename', - # Used for storing xml attributes between import and export, for roundtrips - 'xml_attributes') + 'tabs', 'grading_policy', 'published_by', 'published_date', + 'discussion_blackouts', 'testcenter_info', + # VS[compat] -- remove the below attrs once everything is in the CMS + 'course', 'org', 'url_name', 'filename', + # Used for storing xml attributes between import and export, for roundtrips + 'xml_attributes') metadata_to_export_to_policy = ('discussion_topics') @@ -166,7 +176,7 @@ class XmlDescriptor(XModuleDescriptor): for field in set(cls.fields + cls.lms.fields): if field.name == attr: from_xml = lambda val: deserialize_field(field, val) - to_xml = lambda val : serialize_field(val) + to_xml = lambda val: serialize_string_literal(val) if isinstance(val, basestring) else serialize_field(val) return AttrMap(from_xml, to_xml) return AttrMap() @@ -254,7 +264,7 @@ class XmlDescriptor(XModuleDescriptor): definition, children = cls.definition_from_xml(definition_xml, system) if definition_metadata: definition['definition_metadata'] = definition_metadata - definition['filename'] = [ filepath, filename ] + definition['filename'] = [filepath, filename] return definition, children @@ -280,7 +290,6 @@ class XmlDescriptor(XModuleDescriptor): metadata[attr] = attr_map.from_xml(val) return metadata - @classmethod def apply_policy(cls, metadata, policy): """ @@ -374,7 +383,6 @@ class XmlDescriptor(XModuleDescriptor): """ return True - def export_to_xml(self, resource_fs): """ Returns an xml string representing this module, and all modules From 5aae3c69715bb84294354299f26fbcf07388c0d6 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 12 Jul 2013 11:14:35 -0400 Subject: [PATCH 2/6] add new tests for the string literal serialization --- .../xmodule/xmodule/tests/test_xml_module.py | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py index 6581ce58f6..a853e6ab3f 100644 --- a/common/lib/xmodule/xmodule/tests/test_xml_module.py +++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py @@ -4,7 +4,7 @@ from xmodule.x_module import XModuleFields from xblock.core import Scope, String, Dict, Boolean, Integer, Float, Any, List from xmodule.fields import Date, Timedelta -from xmodule.xml_module import XmlDescriptor, serialize_field, deserialize_field +from xmodule.xml_module import XmlDescriptor, serialize_field, deserialize_field, serialize_string_literal import unittest from .import get_test_system from nose.tools import assert_equals @@ -186,12 +186,27 @@ class TestSerialize(unittest.TestCase): assert_equals('"false"', serialize_field('false')) assert_equals('"fAlse"', serialize_field('fAlse')) assert_equals('"hat box"', serialize_field('hat box')) - assert_equals('{"bar": "hat", "frog": "green"}', serialize_field({'bar': 'hat', 'frog' : 'green'})) + assert_equals('{"bar": "hat", "frog": "green"}', serialize_field({'bar': 'hat', 'frog': 'green'})) assert_equals('[3.5, 5.6]', serialize_field([3.5, 5.6])) assert_equals('["foo", "bar"]', serialize_field(['foo', 'bar'])) assert_equals('"2012-12-31T23:59:59Z"', serialize_field("2012-12-31T23:59:59Z")) assert_equals('"1 day 12 hours 59 minutes 59 seconds"', - serialize_field("1 day 12 hours 59 minutes 59 seconds")) + serialize_field("1 day 12 hours 59 minutes 59 seconds")) + + def test_serialize_string_literal(self): + assert_equals('2', serialize_string_literal('2')) + assert_equals('2.589', serialize_string_literal('2.589')) + assert_equals('false', serialize_string_literal('false')) + assert_equals('fAlse', serialize_string_literal('fAlse')) + assert_equals('hat box', serialize_string_literal('hat box')) + assert_equals('2012-12-31T23:59:59Z', serialize_string_literal("2012-12-31T23:59:59Z")) + assert_equals('1 day 12 hours 59 minutes 59 seconds', + serialize_string_literal("1 day 12 hours 59 minutes 59 seconds")) + + try: + self.assertRaises(serialize_string_literal(2.31)) + except Exception: + pass class TestDeserialize(unittest.TestCase): From ab4012cc594341ac78663bd89f90621cb8c02bd3 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 12 Jul 2013 11:33:46 -0400 Subject: [PATCH 3/6] fix up some of the test courses which had hardcoded that quot; escaping --- common/test/data/simple/course.xml | 8 ++++---- .../test/data/test_exam_registration/course/2012_Fall.xml | 4 ++-- common/test/data/test_start_date/course/2012_Fall.xml | 4 ++-- common/test/data/toy/chapter/secret/magic.xml | 2 +- common/test/data/toy/course/2012_Fall.xml | 8 ++++---- common/test/data/two_toys/video/Video_Resources.xml | 2 +- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/common/test/data/simple/course.xml b/common/test/data/simple/course.xml index c9bb5ec8b2..529528ca0a 100644 --- a/common/test/data/simple/course.xml +++ b/common/test/data/simple/course.xml @@ -1,13 +1,13 @@ -