From 7d0687ec73c6d94ccdd9a77ee21ac82bca9a065b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastien=20G=C3=A9rard?= Date: Sat, 11 May 2019 22:34:25 +0200 Subject: [PATCH] custom field validator is now expected to raise a ValidationError (drop support for returning True/False) --- docs/changelog.rst | 2 ++ mongoengine/base/fields.py | 21 ++++++++++++------- mongoengine/errors.py | 5 +++++ tests/fields/fields.py | 43 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 7 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 6ac8da93..6864627b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -5,6 +5,8 @@ Changelog Development =========== - Add support for MongoDB 3.6 and Python3.7 in travis +- BREAKING CHANGE: Changed the custom field validator (i.e `validation` parameter of Field) so that it now requires: + the callable to raise a ValidationError (i.o return True/False). - Fix querying on List(EmbeddedDocument) subclasses fields #1961 #1492 - Fix querying on (Generic)EmbeddedDocument subclasses fields #475 - expose `mongoengine.connection.disconnect` and `mongoengine.connection.disconnect_all` diff --git a/mongoengine/base/fields.py b/mongoengine/base/fields.py index 598eb606..5962df14 100644 --- a/mongoengine/base/fields.py +++ b/mongoengine/base/fields.py @@ -11,8 +11,7 @@ from mongoengine.base.common import UPDATE_OPERATORS from mongoengine.base.datastructures import (BaseDict, BaseList, EmbeddedDocumentList) from mongoengine.common import _import_class -from mongoengine.errors import ValidationError - +from mongoengine.errors import ValidationError, DeprecatedError __all__ = ('BaseField', 'ComplexBaseField', 'ObjectIdField', 'GeoJsonBaseField') @@ -53,8 +52,8 @@ class BaseField(object): unique with. :param primary_key: Mark this field as the primary key. Defaults to False. :param validation: (optional) A callable to validate the value of the - field. Generally this is deprecated in favour of the - `FIELD.validate` method + field. The callable takes the value as parameter and should raise + a ValidationError if validation fails :param choices: (optional) The valid choices :param null: (optional) If the field value can be null. If no and there is a default value then the default value is set @@ -226,10 +225,18 @@ class BaseField(object): # check validation argument if self.validation is not None: if callable(self.validation): - if not self.validation(value): - self.error('Value does not match custom validation method') + try: + # breaking change of 0.18 + # Get rid of True/False-type return for the validation method + # in favor of having validation raising a ValidationError + ret = self.validation(value) + if ret is not None: + raise DeprecatedError('validation argument for `%s` must not return anything, ' + 'it should raise a ValidationError if validation fails' % self.name) + except ValidationError as ex: + self.error(str(ex)) else: - raise ValueError('validation argument for "%s" must be a ' + raise ValueError('validation argument for `"%s"` must be a ' 'callable.' % self.name) self.validate(value, **kwargs) diff --git a/mongoengine/errors.py b/mongoengine/errors.py index b0009cbc..4aecef5e 100644 --- a/mongoengine/errors.py +++ b/mongoengine/errors.py @@ -142,3 +142,8 @@ class ValidationError(AssertionError): for k, v in iteritems(self.to_dict()): error_dict[generate_key(v)].append(k) return ' '.join(['%s: %s' % (k, v) for k, v in iteritems(error_dict)]) + + +class DeprecatedError(Exception): + """Raise when a user uses a feature that has been Deprecated""" + pass diff --git a/tests/fields/fields.py b/tests/fields/fields.py index 5eaee4be..68baab46 100644 --- a/tests/fields/fields.py +++ b/tests/fields/fields.py @@ -12,6 +12,7 @@ from mongoengine import Document, StringField, IntField, DateTimeField, DateFiel FieldDoesNotExist, EmbeddedDocumentListField, MultipleObjectsReturned, NotUniqueError, BooleanField,\ ObjectIdField, SortedListField, GenericLazyReferenceField, LazyReferenceField, DynamicDocument from mongoengine.base import (BaseField, EmbeddedDocumentList, _document_registry) +from mongoengine.errors import DeprecatedError from tests.utils import MongoDBTestCase @@ -56,6 +57,48 @@ class FieldTest(MongoDBTestCase): self.assertEqual( data_to_be_saved, ['age', 'created', 'day', 'name', 'userid']) + def test_custom_field_validation_raise_deprecated_error_when_validation_return_something(self): + # Covers introduction of a breaking change in the validation parameter (0.18) + def _not_empty(z): + return bool(z) + + class Person(Document): + name = StringField(validation=_not_empty) + + Person.drop_collection() + + error = ("validation argument for `name` must not return anything, " + "it should raise a ValidationError if validation fails") + + with self.assertRaises(DeprecatedError) as ctx_err: + Person(name="").validate() + self.assertEqual(str(ctx_err.exception), error) + + with self.assertRaises(DeprecatedError) as ctx_err: + Person(name="").save() + self.assertEqual(str(ctx_err.exception), error) + + def test_custom_field_validation_raise_validation_error(self): + def _not_empty(z): + if not z: + raise ValidationError('cantbeempty') + + class Person(Document): + name = StringField(validation=_not_empty) + + Person.drop_collection() + + with self.assertRaises(ValidationError) as ctx_err: + Person(name="").validate() + self.assertEqual("ValidationError (Person:None) (cantbeempty: ['name'])", str(ctx_err.exception)) + + with self.assertRaises(ValidationError): + Person(name="").save() + self.assertEqual("ValidationError (Person:None) (cantbeempty: ['name'])", str(ctx_err.exception)) + + Person(name="garbage").validate() + Person(name="garbage").save() + def test_default_values_set_to_None(self): """Ensure that default field values are used even when we explcitly initialize the doc with None values.