diff --git a/xblock/fields.py b/xblock/fields.py index d9f672a..7d44611 100644 --- a/xblock/fields.py +++ b/xblock/fields.py @@ -439,7 +439,11 @@ def _check_or_enforce_type(self, value): value, traceback.format_exc().splitlines()[-1]) warnings.warn(message, FailingEnforceTypeWarning, stacklevel=3) else: - if value != new_value: + try: + equal = value == new_value + except TypeError: + equal = False + if not equal: message = u"The value {} would be enforced to {}".format( value, new_value) warnings.warn(message, ModifyingEnforceTypeWarning, stacklevel=3) @@ -598,7 +602,7 @@ def from_string(self, serialized): """ self._warn_deprecated_outside_JSONField() value = yaml.safe_load(serialized) - return self._check_or_enforce_type(value) + return self.enforce_type(value) def enforce_type(self, value): """ @@ -838,31 +842,27 @@ def from_json(self, value): """ Parse the date from an ISO-formatted date string, or None. """ - if isinstance(value, basestring): + if value is None: + return None + if isinstance(value, basestring): # Parser interprets empty string as now by default if value == "": return None - try: - parsed_date = dateutil.parser.parse(value) + value = dateutil.parser.parse(value) except (TypeError, ValueError): raise ValueError("Could not parse {} as a date".format(value)) - if parsed_date.tzinfo is not None: # pylint: disable=maybe-no-member - parsed_date.astimezone(pytz.utc) # pylint: disable=maybe-no-member - else: - parsed_date = parsed_date.replace(tzinfo=pytz.utc) # pylint: disable=maybe-no-member - - return parsed_date - - if value is None: - return None - - if isinstance(value, datetime.datetime): - return value + if not isinstance(value, datetime.datetime): + raise TypeError( + "Value should be loaded from a string, a datetime object or None, not {}".format(type(value)) + ) - raise TypeError("Value should be loaded from a string, not {}".format(type(value))) + if value.tzinfo is not None: # pylint: disable=maybe-no-member + return value.astimezone(pytz.utc) # pylint: disable=maybe-no-member + else: + return value.replace(tzinfo=pytz.utc) # pylint: disable=maybe-no-member def to_json(self, value): """ @@ -874,11 +874,11 @@ def to_json(self, value): return None raise TypeError("Value stored must be a datetime object, not {}".format(type(value))) - def enforce_type(self, value): - if isinstance(value, datetime.datetime) or value is None: - return value + def to_string(self, value): + """DateTime fields get serialized without quote marks.""" + return self.to_json(value) - return self.from_json(value) + enforce_type = from_json class Any(JSONField): diff --git a/xblock/test/test_fields.py b/xblock/test/test_fields.py index 2e5cbc2..e272b7b 100644 --- a/xblock/test/test_fields.py +++ b/xblock/test/test_fields.py @@ -37,9 +37,9 @@ class FieldTest(unittest.TestCase): FIELD_TO_TEST = Mock() - def set_and_get_field(self, arg, enforce_type): + def get_block(self, enforce_type): """ - Set the field to arg in a Block, get it and return it + Create a block with a field 'field_x' that is of type FIELD_TO_TEST. """ class TestBlock(XBlock): """ @@ -48,7 +48,13 @@ class TestBlock(XBlock): field_x = self.FIELD_TO_TEST(enforce_type=enforce_type) runtime = TestRuntime(services={'field-data': DictFieldData({})}) - block = TestBlock(runtime, scope_ids=Mock(spec=ScopeIds)) + return TestBlock(runtime, scope_ids=Mock(spec=ScopeIds)) + + def set_and_get_field(self, arg, enforce_type): + """ + Set the field to arg in a Block, get it and return the set value. + """ + block = self.get_block(enforce_type) block.field_x = arg return block.field_x @@ -223,6 +229,7 @@ def test_error(self): self.assertJSONOrSetTypeError({}) +@ddt.ddt class DateTest(FieldTest): """ Tests of the Date field. @@ -258,6 +265,26 @@ def test_serialize(self): dt.datetime(2014, 4, 1, 2, 3, 4).replace(tzinfo=pytz.utc) ) + @ddt.unpack + @ddt.data( + ( + dt.datetime(2014, 4, 1, 2, 3, 4).replace(tzinfo=pytz.utc), + dt.datetime(2014, 4, 1, 2, 3, 5) + ), + ( + dt.datetime(2014, 4, 1, 2, 3, 4), + dt.datetime(2014, 4, 1, 2, 3, 4).replace(tzinfo=pytz.utc), + ) + ) + def test_naive(self, original, replacement): + """ + Make sure field comparison doesn't crash when comparing naive and non-naive datetimes. + """ + for enforce_type in (False, True): + block = self.get_block(enforce_type) + block.field_x = original + block.field_x = replacement + def test_none(self): self.assertJSONOrSetEquals(None, None) self.assertJSONOrSetEquals(None, '') @@ -652,16 +679,18 @@ def assert_to_string(self, _type, value, string): """ Helper method: checks if _type's to_string given instance of _type returns expected string """ - result = _type(enforce_type=True).to_string(value) + result = _type().to_string(value) self.assertEquals(result, string) def assert_from_string(self, _type, string, value): """ Helper method: checks if _type's from_string given string representation of type returns expected value """ - result = _type(enforce_type=True).from_string(string) + result = _type().from_string(string) self.assertEquals(result, value) + # Serialisation test data that is tested both ways, i.e. whether serialisation of the value + # yields the string and deserialisation of the string yields the value. @ddt.unpack @ddt.data( (Integer, 0, '0'), @@ -700,7 +729,10 @@ def assert_from_string(self, _type, string, value): 2, 3 ] - }"""))) + }""")), + (DateTime, dt.datetime(2014, 4, 1, 2, 3, 4, 567890).replace(tzinfo=pytz.utc), '2014-04-01T02:03:04.567890'), + (DateTime, dt.datetime(2014, 4, 1, 2, 3, 4).replace(tzinfo=pytz.utc), '2014-04-01T02:03:04.000000'), + ) def test_both_directions(self, _type, value, string): """Easy cases that work in both directions.""" self.assert_to_string(_type, value, string) @@ -712,9 +744,13 @@ def test_both_directions(self, _type, value, string): (Float, 1.0, r"1|1\.0*"), (Float, -10.0, r"-10|-10\.0*")) def test_to_string_regexp_matches(self, _type, value, regexp): - result = _type(enforce_type=True).to_string(value) + result = _type().to_string(value) self.assertRegexpMatches(result, regexp) + # Test data for non-canonical serialisations of values that we should be able to correctly + # deserialise. These values are not serialised to the representation given here for various + # reasons; some of them are non-standard number representations, others are YAML data that + # isn't valid JSON, yet others use non-standard capitalisation. @ddt.unpack @ddt.data( (Integer, "0xff", 0xff), @@ -780,7 +816,11 @@ def test_to_string_regexp_matches(self, _type, value, regexp): kaw: null """), [1, 2.345, {"foo": True, "bar": [1, 2, 3]}, {"meow": False, "woof": True, "kaw": None}] - ) + ), + # Test that legacy DateTime format including double quotes can still be imported for compatibility with + # old data export tar balls. + (DateTime, '"2014-04-01T02:03:04.567890"', dt.datetime(2014, 4, 1, 2, 3, 4, 567890).replace(tzinfo=pytz.utc)), + (DateTime, '"2014-04-01T02:03:04.000000"', dt.datetime(2014, 4, 1, 2, 3, 4).replace(tzinfo=pytz.utc)), ) def test_from_string(self, _type, string, value): self.assert_from_string(_type, string, value) @@ -791,7 +831,7 @@ def test_float_from_NaN_is_nan(self): # pylint: disable=invalid-name This special test case is necessary since float('nan') compares inequal to everything. """ - result = Float(enforce_type=True).from_string('NaN') + result = Float().from_string('NaN') self.assertTrue(math.isnan(result)) @ddt.unpack @@ -801,4 +841,4 @@ def test_float_from_NaN_is_nan(self): # pylint: disable=invalid-name def test_from_string_errors(self, _type, string): """ Cases that raises various exceptions.""" with self.assertRaises(StandardError): - _type(enforce_type=True).from_string(string) + _type().from_string(string)