From 86d952bf91c7f5fa3159d2830ef8b81c6ee09c5e Mon Sep 17 00:00:00 2001 From: cahrens Date: Wed, 12 Jun 2013 13:34:38 -0400 Subject: [PATCH] Change to putting serialize/de-serialize directly in xml_module. --- .../contentstore/tests/test_checklists.py | 43 +++- .../lib/xmodule/xmodule/tests/test_fields.py | 32 --- .../xmodule/xmodule/tests/test_xml_module.py | 190 +++++++++++++++++- common/lib/xmodule/xmodule/xml_module.py | 40 +++- requirements/edx/github.txt | 3 +- 5 files changed, 261 insertions(+), 47 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_checklists.py b/cms/djangoapps/contentstore/tests/test_checklists.py index f0889b0861..54bc726092 100644 --- a/cms/djangoapps/contentstore/tests/test_checklists.py +++ b/cms/djangoapps/contentstore/tests/test_checklists.py @@ -19,6 +19,24 @@ class ChecklistTestCase(CourseTestCase): modulestore = get_modulestore(self.course.location) return modulestore.get_item(self.course.location).checklists + + def compare_checklists(self, persisted, request): + """ + Handles url expansion as possible difference and descends into guts + :param persisted: + :param request: + """ + self.assertEqual(persisted['short_description'], request['short_description']) + compare_urls = (persisted.get('action_urls_expanded') == request.get('action_urls_expanded')) + for pers, req in zip(persisted['items'], request['items']): + self.assertEqual(pers['short_description'], req['short_description']) + self.assertEqual(pers['long_description'], req['long_description']) + self.assertEqual(pers['is_checked'], req['is_checked']) + if compare_urls: + self.assertEqual(pers['action_url'], req['action_url']) + self.assertEqual(pers['action_text'], req['action_text']) + self.assertEqual(pers['action_external'], req['action_external']) + def test_get_checklists(self): """ Tests the get checklists method. """ checklists_url = get_url_reverse('Checklists', self.course) @@ -31,9 +49,9 @@ class ChecklistTestCase(CourseTestCase): self.course.checklists = None modulestore = get_modulestore(self.course.location) modulestore.update_metadata(self.course.location, own_metadata(self.course)) - self.assertEquals(self.get_persisted_checklists(), None) + self.assertEqual(self.get_persisted_checklists(), None) response = self.client.get(checklists_url) - self.assertEquals(payload, response.content) + self.assertEqual(payload, response.content) def test_update_checklists_no_index(self): """ No checklist index, should return all of them. """ @@ -43,7 +61,8 @@ class ChecklistTestCase(CourseTestCase): 'name': self.course.location.name}) returned_checklists = json.loads(self.client.get(update_url).content) - self.assertListEqual(self.get_persisted_checklists(), returned_checklists) + for pay, resp in zip(self.get_persisted_checklists(), returned_checklists): + self.compare_checklists(pay, resp) def test_update_checklists_index_ignored_on_get(self): """ Checklist index ignored on get. """ @@ -53,7 +72,8 @@ class ChecklistTestCase(CourseTestCase): 'checklist_index': 1}) returned_checklists = json.loads(self.client.get(update_url).content) - self.assertListEqual(self.get_persisted_checklists(), returned_checklists) + for pay, resp in zip(self.get_persisted_checklists(), returned_checklists): + self.compare_checklists(pay, resp) def test_update_checklists_post_no_index(self): """ No checklist index, will error on post. """ @@ -78,13 +98,18 @@ class ChecklistTestCase(CourseTestCase): 'course': self.course.location.course, 'name': self.course.location.name, 'checklist_index': 2}) + + def get_first_item(checklist): + return checklist['items'][0] + payload = self.course.checklists[2] - self.assertFalse(payload.get('is_checked')) - payload['is_checked'] = True + self.assertFalse(get_first_item(payload).get('is_checked')) + get_first_item(payload)['is_checked'] = True returned_checklist = json.loads(self.client.post(update_url, json.dumps(payload), "application/json").content) - self.assertTrue(returned_checklist.get('is_checked')) - self.assertEqual(self.get_persisted_checklists()[2], returned_checklist) + self.assertTrue(get_first_item(returned_checklist).get('is_checked')) + pers = self.get_persisted_checklists() + self.compare_checklists(pers[2], returned_checklist) def test_update_checklists_delete_unsupported(self): """ Delete operation is not supported. """ @@ -93,4 +118,4 @@ class ChecklistTestCase(CourseTestCase): 'name': self.course.location.name, 'checklist_index': 100}) response = self.client.delete(update_url) - self.assertContains(response, 'Unsupported request', status_code=400) \ No newline at end of file + self.assertContains(response, 'Unsupported request', status_code=400) diff --git a/common/lib/xmodule/xmodule/tests/test_fields.py b/common/lib/xmodule/xmodule/tests/test_fields.py index d944566e97..41b77d4477 100644 --- a/common/lib/xmodule/xmodule/tests/test_fields.py +++ b/common/lib/xmodule/xmodule/tests/test_fields.py @@ -78,22 +78,6 @@ class DateTest(unittest.TestCase): DateTest.date.from_json("2012-12-31T23:00:01-01:00")), "2013-01-01T00:00:01Z") - def test_serialize(self): - self.assertEqual( - DateTest.date.serialize("2012-12-31T23:59:59Z"), - '"2012-12-31T23:59:59Z"' - ) - - def test_deserialize(self): - self.assertEqual( - '2012-12-31T23:59:59Z', - DateTest.date.deserialize("2012-12-31T23:59:59Z"), - ) - self.assertEqual( - '2012-12-31T23:59:59Z', - DateTest.date.deserialize('"2012-12-31T23:59:59Z"'), - ) - class TimedeltaTest(unittest.TestCase): delta = Timedelta() @@ -114,19 +98,3 @@ class TimedeltaTest(unittest.TestCase): '1 days 46799 seconds', TimedeltaTest.delta.to_json(datetime.timedelta(days=1, hours=12, minutes=59, seconds=59)) ) - - def test_serialize(self): - self.assertEqual( - TimedeltaTest.delta.serialize('1 day 12 hours 59 minutes 59 seconds'), - '"1 day 12 hours 59 minutes 59 seconds"' - ) - - def test_deserialize(self): - self.assertEqual( - '1 day 12 hours 59 minutes 59 seconds', - TimedeltaTest.delta.deserialize('1 day 12 hours 59 minutes 59 seconds') - ) - self.assertEqual( - '1 day 12 hours 59 minutes 59 seconds', - TimedeltaTest.delta.deserialize('"1 day 12 hours 59 minutes 59 seconds"') - ) diff --git a/common/lib/xmodule/xmodule/tests/test_xml_module.py b/common/lib/xmodule/xmodule/tests/test_xml_module.py index 18cd11650f..c45f14b6c2 100644 --- a/common/lib/xmodule/xmodule/tests/test_xml_module.py +++ b/common/lib/xmodule/xmodule/tests/test_xml_module.py @@ -2,11 +2,12 @@ #pylint: disable=C0111 from xmodule.x_module import XModuleFields -from xblock.core import Scope, String, Object, Boolean, Integer, Float -from xmodule.fields import Date -from xmodule.xml_module import XmlDescriptor +from xblock.core import Scope, String, Object, Boolean, Integer, Float, Any, List +from xmodule.fields import Date, Timedelta +from xmodule.xml_module import XmlDescriptor, serialize_field, deserialize_field import unittest from .import test_system +from nose.tools import assert_equals from mock import Mock @@ -171,3 +172,186 @@ class EditableMetadataFieldsTest(unittest.TestCase): self.assertEqual(explicitly_set, test_field['explicitly_set']) self.assertEqual(inheritable, test_field['inheritable']) + + +def assertSerializeEqual(expected, arg): + """ + Asserts the result of serialize_field. + """ + assert_equals(expected, serialize_field(arg)) + + +def assertDeserializeEqual(field, expected, arg): + """ + Asserts the result of deserialize_field. + """ + assert_equals(expected, deserialize_field(field, arg)) + + +class TestSerializeInteger(unittest.TestCase): + """ Tests serialize/deserialize as related to Integer type. """ + + def test_serialize(self): + assertSerializeEqual('-2', -2) + assertSerializeEqual('"2"', '2') + assertSerializeEqual('null', None) + + def test_deserialize(self): + assertDeserializeEqual(Integer(), None, 'null') + assertDeserializeEqual(Integer(), -2, '-2') + assertDeserializeEqual(Integer(), "450", '"450"') + + # False can be parsed as a int (converts to 0) + assertDeserializeEqual(Integer(), False, 'false') + # True can be parsed as a int (converts to 1) + assertDeserializeEqual(Integer(), True, 'true') + + def test_deserialize_unsupported_types(self): + assertDeserializeEqual(Integer(), '[3]', '[3]') + self.assertRaises(TypeError, deserialize_field, None) + + +class FloatTest(unittest.TestCase): + """ Tests serialize/deserialize as related to Float type. """ + + def test_serialize(self): + assertSerializeEqual('-2', -2) + assertSerializeEqual('"2"', '2') + assertSerializeEqual('-3.41', -3.41) + assertSerializeEqual('"2.589"', '2.589') + assertSerializeEqual('null', None) + + def test_deserialize(self): + assertDeserializeEqual(Float(), None, 'null') + assertDeserializeEqual(Float(), -2, '-2') + assertDeserializeEqual(Float(), "450", '"450"') + assertDeserializeEqual(Float(), -2.78, '-2.78') + assertDeserializeEqual(Float(), "0.45", '"0.45"') + + # False can be parsed as a float (converts to 0) + assertDeserializeEqual(Float(), False, 'false') + # True can be parsed as a float (converts to 1) + assertDeserializeEqual(Float(), True, 'true') + + def test_deserialize_unsupported_types(self): + assertDeserializeEqual(Float(), '[3]', '[3]') + self.assertRaises(TypeError, deserialize_field, None) + + +class BooleanTest(unittest.TestCase): + """ Tests serialize/deserialize as related to Boolean type. """ + + def test_serialize(self): + assertSerializeEqual('false', False) + assertSerializeEqual('"false"', 'false') + assertSerializeEqual('"fAlse"', 'fAlse') + assertSerializeEqual('null', None) + + def test_deserialize(self): + # json.loads converts the value to Python bool + assertDeserializeEqual(Boolean(), False, 'false') + assertDeserializeEqual(Boolean(), True, 'true') + + # json.loads fails, string value is returned. + assertDeserializeEqual(Boolean(), 'False', 'False') + assertDeserializeEqual(Boolean(), 'True', 'True') + + # json.loads deserializes 'null' to None + assertDeserializeEqual(Boolean(), None, 'null') + + # json.loads deserializes as a string + assertDeserializeEqual(Boolean(), 'false', '"false"') + assertDeserializeEqual(Boolean(), 'fAlse', '"fAlse"') + assertDeserializeEqual(Boolean(), "TruE", '"TruE"') + + def test_deserialize_unsupported_types(self): + self.assertRaises(TypeError, deserialize_field, None) + + +class StringTest(unittest.TestCase): + """ Tests serialize/deserialize as related to String type. """ + + def test_serialize(self): + assertSerializeEqual('"hat box"', 'hat box') + assertSerializeEqual('null', None) + + def test_deserialize(self): + assertDeserializeEqual(String(), 'hAlf', '"hAlf"') + assertDeserializeEqual(String(), 'false', '"false"') + assertDeserializeEqual(String(), 'single quote', 'single quote') + assertDeserializeEqual(String(), None, 'null') + + def test_deserialize_unsupported_types(self): + assertDeserializeEqual(String(), '3.4', '3.4') + assertDeserializeEqual(String(), 'false', 'false') + assertDeserializeEqual(String(), '2', '2') + assertDeserializeEqual(String(), '[3]', '[3]') + self.assertRaises(TypeError, deserialize_field, None) + + +class AnyTest(unittest.TestCase): + """ Tests serialize/deserialize as related to Any type. """ + + def test_serialize(self): + assertSerializeEqual('{"bar": "hat", "frog": "green"}', {'bar': 'hat', 'frog' : 'green'}) + assertSerializeEqual('[3.5, 5.6]', [3.5, 5.6]) + assertSerializeEqual('"hat box"', 'hat box') + assertSerializeEqual('null', None) + + def test_deserialize(self): + assertDeserializeEqual(Any(), 'hAlf', '"hAlf"') + assertDeserializeEqual(Any(), 'false', '"false"') + assertDeserializeEqual(Any(), None, 'null') + assertDeserializeEqual(Any(), {'bar': 'hat', 'frog' : 'green'}, '{"bar": "hat", "frog": "green"}') + assertDeserializeEqual(Any(), [3.5, 5.6], '[3.5, 5.6]') + assertDeserializeEqual(Any(), '[', '[') + assertDeserializeEqual(Any(), False, 'false') + assertDeserializeEqual(Any(), 3.4, '3.4') + + def test_deserialize_unsupported_types(self): + self.assertRaises(TypeError, deserialize_field, None) + + +class ListTest(unittest.TestCase): + """ Tests serialize/deserialize as related to Any type. """ + + def test_serialize(self): + assertSerializeEqual('["foo", "bar"]', ['foo', 'bar']) + assertSerializeEqual('[3.5, 5.6]', [3.5, 5.6]) + assertSerializeEqual('null', None) + + def test_deserialize(self): + assertDeserializeEqual(List(), ['foo', 'bar'], '["foo", "bar"]') + assertDeserializeEqual(List(), [3.5, 5.6], '[3.5, 5.6]') + assertDeserializeEqual(List(), [], '[]') + assertDeserializeEqual(List(), None, 'null') + + def test_deserialize_unsupported_types(self): + assertDeserializeEqual(List(), '3.4', '3.4') + assertDeserializeEqual(List(), 'false', 'false') + assertDeserializeEqual(List(), '2', '2') + self.assertRaises(TypeError, deserialize_field, None) + + +class DateTest(unittest.TestCase): + """ Tests serialize/deserialize as related to Date type. """ + + def test_serialize(self): + assertSerializeEqual('"2012-12-31T23:59:59Z"', "2012-12-31T23:59:59Z") + + def test_deserialize(self): + assertDeserializeEqual(Date(), '2012-12-31T23:59:59Z', "2012-12-31T23:59:59Z") + assertDeserializeEqual(Date(), '2012-12-31T23:59:59Z', '"2012-12-31T23:59:59Z"') + + +class TimedeltaTest(unittest.TestCase): + + def test_serialize(self): + assertSerializeEqual('"1 day 12 hours 59 minutes 59 seconds"', + "1 day 12 hours 59 minutes 59 seconds") + + def test_deserialize(self): + assertDeserializeEqual(Timedelta(), '1 day 12 hours 59 minutes 59 seconds', + '1 day 12 hours 59 minutes 59 seconds') + assertDeserializeEqual(Timedelta(), '1 day 12 hours 59 minutes 59 seconds', + '"1 day 12 hours 59 minutes 59 seconds"') diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 56f8b6fd15..0417dfc0a6 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -79,6 +79,42 @@ class AttrMap(_AttrMapBase): return _AttrMapBase.__new__(_cls, from_xml, to_xml) +def serialize_field(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. + """ + return json.dumps(value) + + +def deserialize_field(field, value): + """ + Deserialize the string version to the value stored internally. + + Note that this is not the same as the value returned by from_json, as model types typically store + their value internally as JSON. By default, this method will return the result of calling json.loads + on the supplied value, unless json.loads throws a TypeError, or the type of the value returned by json.loads + is not supported for this class (see 'is_type_supported'). In either of those cases, this method returns + the input value. + """ + try: + deserialized = json.loads(value) + + if deserialized is None: + return deserialized + try: + field.from_json(deserialized) + return deserialized + except (ValueError, TypeError): + # Support older serialized forms by simply returning the String representation + return value + + except (ValueError, TypeError): + # Support older serialized version, which was just the String (not the result of json.dumps). + return value + + class XmlDescriptor(XModuleDescriptor): """ Mixin class for standardized parsing of from xml @@ -124,8 +160,8 @@ class XmlDescriptor(XModuleDescriptor): def get_map_for_field(cls, attr): for field in set(cls.fields + cls.lms.fields): if field.name == attr: - from_xml = lambda val: field.deserialize(val) - to_xml = lambda val : field.serialize(val) + from_xml = lambda val: deserialize_field(field, val) + to_xml = lambda val : serialize_field(val) return AttrMap(from_xml, to_xml) return AttrMap() diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 36fd9dca06..310807bc55 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -8,6 +8,7 @@ -e git://github.com/eventbrite/zendesk.git@d53fe0e81b623f084e91776bcf6369f8b7b63879#egg=zendesk # Our libraries: --e git+https://github.com/edx/XBlock.git@4c5d2397#egg=XBlock + +-e git+https://github.com/edx/XBlock.git@2abd2937e134e2f78dfa118d6cfd56ec510fd3a4#egg=XBlock -e git+https://github.com/edx/codejail.git@5fb5fa0#egg=codejail -e git+https://github.com/edx/diff-cover.git@v0.1.0#egg=diff_cover