Merge pull request openedx#28 from edx/feature/christina/from_json

Be strict about ModelType expected types.

setup.py

@@ -2,7 +2,7 @@
 
 setup(
     name='XBlock',
-    version='0.1',
+    version='0.2',
     description='XBlock Core Library',
     packages=['xblock'],
     install_requires=[

xblock/core.py

@@ -222,42 +222,114 @@ def __eq__(self, other):
 
 
 class Integer(ModelType):
-    pass
+    """
+    A model type that contains an integer.
+
+    The value, as stored, can be None, '' (which will be treated as None), a Python integer,
+    or a value that will parse as an integer, ie., something for which int(value) does not throw an Error.
+
+    Note that a floating point value will convert to an integer, but a string containing a floating point
+    number ('3.48') will throw an Error.
+    """
+    def from_json(self, value):
+        if value is None or value == '':
+            return None
+        return int(value)
 
 
 class Float(ModelType):
-    pass
+    """
+    A model type that contains a float.
+
+    The value, as stored, can be None, '' (which will be treated as None), a Python float,
+    or a value that will parse as an float, ie., something for which float(value) does not throw an Error.
+    """
+    def from_json(self, value):
+        if value is None or value == '':
+            return None
+        return float(value)
 
 
 class Boolean(ModelType):
     """
-    A field class for representing a Boolean. This class has the values property defined.
+    A field class for representing a boolean.
+
+    The stored value can be either a Python bool, a string, or any value that will then be converted
+    to a bool in the from_json method.
+
+    Examples:
+        True -> True
+        'true' -> True
+        'TRUE' -> True
+        'any other string' -> False
+        [] -> False
+        ['123'] -> True
+        None - > False
+
+    This class has the 'values' property defined.
     """
     def __init__(self, help=None, default=None, scope=Scope.content, display_name=None):
         super(Boolean, self).__init__(help, default, scope, display_name,
             values=({'display_name': "True", "value": True}, {'display_name': "False", "value": False}))
 
+    def from_json(self, value):
+        if isinstance(value, basestring):
+            return value.lower() == 'true'
+        else:
+            return bool(value)
 
-class Object(ModelType):
+
+class Dict(ModelType):
+    """
+    A field class for representing a Python dict.
+
+    The stored value must be either be None or a dict.
+    """
     @property
     def default(self):
         if self._default is None:
             return {}
         else:
             return self._default
 
+    def from_json(self, value):
+        if value is None or isinstance(value, dict):
+            return value
+        else:
+            raise TypeError('Value stored in a Dict must be None or a dict.')
+
 
 class List(ModelType):
+    """
+    A field class for representing a list.
+
+    The stored value can either be None or a list.
+    """
     @property
     def default(self):
         if self._default is None:
             return []
         else:
             return self._default
 
+    def from_json(self, value):
+        if value is None or isinstance(value, list):
+            return value
+        else:
+            raise TypeError('Value stored in an Object must be None or a list.')
+
 
 class String(ModelType):
-    pass
+    """
+    A field class for representing a string.
+
+    The stored value can either be None or a basestring instance.
+    """
+    def from_json(self, value):
+        if value is None or isinstance(value, basestring):
+            return value
+        else:
+            raise TypeError('Value stored in a String must be None or a String.')
 
 
 class Any(ModelType):
@@ -413,7 +485,7 @@ class XBlock(Plugin):
 
     entry_point = 'xblock.v1'
 
-    parent = Object(help='The id of the parent of this XBlock', default=None, scope=Scope.parent)
+    parent = String(help='The id of the parent of this XBlock', default=None, scope=Scope.parent)
     name = String(help="Short name for the block", scope=Scope.settings)
     tags = List(help="Tags for this block", scope=Scope.settings)
 

xblock/problem.py

@@ -35,7 +35,7 @@
 import string
 import time
 
-from .core import XBlock, Integer, Object, Scope, String, Any, Boolean
+from .core import XBlock, Integer, Scope, String, Any, Boolean, Dict
 from .run_script import run_script
 from .fragment import Fragment
 
@@ -242,7 +242,7 @@ def submit(self, submission):
 @XBlock.tag("checker")
 class CheckerBlock(XBlock):
 
-    arguments = Object(help="The arguments expected by `check`")
+    arguments = Dict(help="The arguments expected by `check`")
 
     @classmethod
     def preprocess_input(cls, node, usage_factory):

xblock/test/test_model_types.py

@@ -0,0 +1,208 @@
+"""
+Tests for classes extending ModelType.
+"""
+
+import unittest
+from xblock.core import *
+
+
+class ModelTypeTest(unittest.TestCase):
+    """ Base test class for ModelTypes. """
+
+    def assertJSONEquals(self, expected, arg):
+        """
+        Asserts the result of field.from_json.
+        """
+        self.assertEqual(expected, self.test_field().from_json(arg))
+
+
+    def assertJSONValueError(self, arg):
+        """
+        Asserts that field.from_json throws a ValueError for the supplied value.
+        """
+        with self.assertRaises(ValueError):
+            self.test_field().from_json(arg)
+
+
+    def assertJSONTypeError(self, arg):
+        """
+        Asserts that field.from_json throws a TypeError for the supplied value.
+        """
+        with self.assertRaises(TypeError):
+            self.test_field().from_json(arg)
+
+
+class IntegerTest(ModelTypeTest):
+    """
+    Tests the Integer ModelType.
+    """
+    test_field = Integer
+
+    def test_integer(self):
+        self.assertJSONEquals(5, '5')
+        self.assertJSONEquals(0, '0')
+        self.assertJSONEquals(-1023, '-1023')
+        self.assertJSONEquals(7, 7)
+        self.assertJSONEquals(0, False)
+        self.assertJSONEquals(1, True)
+
+    def test_float_converts(self):
+        self.assertJSONEquals(1, 1.023)
+        self.assertJSONEquals(-3, -3.8)
+
+    def test_none(self):
+        self.assertJSONEquals(None, None)
+        self.assertJSONEquals(None, '')
+
+    def test_error(self):
+        self.assertJSONValueError('abc')
+        self.assertJSONValueError('[1]')
+        self.assertJSONValueError('1.023')
+
+        self.assertJSONTypeError([])
+        self.assertJSONTypeError({})
+
+
+class FloatTest(ModelTypeTest):
+    """
+    Tests the Float ModelType.
+    """
+    test_field = Float
+
+    def test_float(self):
+        self.assertJSONEquals(.23, '.23')
+        self.assertJSONEquals(5, '5')
+        self.assertJSONEquals(0, '0.0')
+        self.assertJSONEquals(-1023.22, '-1023.22')
+        self.assertJSONEquals(0, 0.0)
+        self.assertJSONEquals(4, 4)
+        self.assertJSONEquals(-0.23, -0.23)
+        self.assertJSONEquals(0, False)
+        self.assertJSONEquals(1, True)
+
+    def test_none(self):
+        self.assertJSONEquals(None, None)
+        self.assertJSONEquals(None, '')
+
+    def test_error(self):
+        self.assertJSONValueError('abc')
+        self.assertJSONValueError('[1]')
+
+        self.assertJSONTypeError([])
+        self.assertJSONTypeError({})
+
+
+class BooleanTest(ModelTypeTest):
+    """
+    Tests the Boolean ModelType.
+    """
+    test_field = Boolean
+
+    def test_false(self):
+        self.assertJSONEquals(False, "false")
+        self.assertJSONEquals(False, "False")
+        self.assertJSONEquals(False, "")
+        self.assertJSONEquals(False, "any other string")
+        self.assertJSONEquals(False, False)
+
+    def test_true(self):
+        self.assertJSONEquals(True, "true")
+        self.assertJSONEquals(True, "TruE")
+        self.assertJSONEquals(True, True)
+
+    def test_none(self):
+        self.assertJSONEquals(False, None)
+
+    def test_everything_converts_to_bool(self):
+        self.assertJSONEquals(True, 123)
+        self.assertJSONEquals(True, ['a'])
+        self.assertJSONEquals(False, [])
+
+
+class StringTest(ModelTypeTest):
+    """
+    Tests the String ModelType.
+    """
+    test_field = String
+
+    def test_json_equals(self):
+        self.assertJSONEquals("false", "false")
+        self.assertJSONEquals("abba", "abba")
+        self.assertJSONEquals('"abba"', '"abba"')
+        self.assertJSONEquals('', '')
+
+    def test_none(self):
+        self.assertJSONEquals(None, None)
+
+    def test_error(self):
+        self.assertJSONTypeError(['a'])
+        self.assertJSONTypeError(1.023)
+        self.assertJSONTypeError(3)
+        self.assertJSONTypeError([1])
+        self.assertJSONTypeError([])
+        self.assertJSONTypeError({})
+
+
+class AnyTest(ModelTypeTest):
+    """
+    Tests the Any ModelType.
+    """
+    test_field = Any
+
+    def test_json_equals(self):
+        self.assertJSONEquals({'bar'}, {'bar'})
+        self.assertJSONEquals("abba", "abba")
+        self.assertJSONEquals('', '')
+        self.assertJSONEquals('3.2', '3.2')
+        self.assertJSONEquals(False, False)
+        self.assertJSONEquals([3, 4], [3, 4])
+
+    def test_none(self):
+        self.assertJSONEquals(None, None)
+
+
+class ListTest(ModelTypeTest):
+    """
+    Tests the List ModelType.
+    """
+    test_field = List
+
+    def test_json_equals(self):
+        self.assertJSONEquals([], [])
+        self.assertJSONEquals(['foo', 'bar'], ['foo', 'bar'])
+        self.assertJSONEquals([1, 3.4], [1, 3.4])
+
+    def test_none(self):
+        self.assertJSONEquals(None, None)
+
+    def test_error(self):
+        self.assertJSONTypeError('abc')
+        self.assertJSONTypeError('')
+        self.assertJSONTypeError('1.23')
+        self.assertJSONTypeError('true')
+        self.assertJSONTypeError(3.7)
+        self.assertJSONTypeError(True)
+        self.assertJSONTypeError({})
+
+
+class DictTest(ModelTypeTest):
+    """
+    Tests the Dict ModelType.
+    """
+    test_field = Dict
+
+    def test_json_equals(self):
+        self.assertJSONEquals({}, {})
+        self.assertJSONEquals({'a' : 'b', 'c' : 3}, {'a' : 'b', 'c' : 3})
+
+    def test_none(self):
+        self.assertJSONEquals(None, None)
+
+    def test_error(self):
+        self.assertJSONTypeError(['foo', 'bar'])
+        self.assertJSONTypeError([])
+        self.assertJSONTypeError('abc')
+        self.assertJSONTypeError('1.23')
+        self.assertJSONTypeError('true')
+        self.assertJSONTypeError(3.7)
+        self.assertJSONTypeError(True)