From 1966054febbb96b713db27513617eabdbd70957b Mon Sep 17 00:00:00 2001 From: Loic Bistuer Date: Tue, 1 Jul 2014 20:48:00 +0700 Subject: [PATCH] Fixed #22915 -- Document backward incompatible changes in the ValidationError constructor. This patch also fixes update_error_dict to better handle the use case described in this ticket, previously the type of the provided container could be lost in some conditions. Thanks Russell Keith-Magee for the report and Tim Graham for review. --- django/core/exceptions.py | 9 +- docs/releases/1.7.txt | 136 ++++++++++++++++++++++++-- tests/forms_tests/tests/test_forms.py | 47 +++++++++ 3 files changed, 177 insertions(+), 15 deletions(-) diff --git a/django/core/exceptions.py b/django/core/exceptions.py index 552b0067c3..28e712e0a5 100644 --- a/django/core/exceptions.py +++ b/django/core/exceptions.py @@ -142,13 +142,10 @@ class ValidationError(Exception): def update_error_dict(self, error_dict): if hasattr(self, 'error_dict'): - if error_dict: - for field, errors in self.error_dict.items(): - error_dict.setdefault(field, []).extend(errors) - else: - error_dict = self.error_dict + for field, error_list in self.error_dict.items(): + error_dict.setdefault(field, []).extend(error_list) else: - error_dict[NON_FIELD_ERRORS] = self.error_list + error_dict.setdefault(NON_FIELD_ERRORS, []).extend(self.error_list) return error_dict def __iter__(self): diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt index b738d16b00..dc42d57e57 100644 --- a/docs/releases/1.7.txt +++ b/docs/releases/1.7.txt @@ -279,6 +279,74 @@ example ``qs.filter(author__birthdate__year__lte=1981)``. For more information about both custom lookups and transforms refer to :doc:`custom lookups ` documentation. +Improvements to ``Form`` error handling +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +``Form.add_error()`` +^^^^^^^^^^^^^^^^^^^^ + +Previously there were two main patterns for handling errors in forms: + +* Raising a :exc:`~django.core.exceptions.ValidationError` from within certain + functions (e.g. ``Field.clean()``, ``Form.clean_()``, or + ``Form.clean()`` for non-field errors.) + +* Fiddling with ``Form._errors`` when targeting a specific field in + ``Form.clean()`` or adding errors from outside of a "clean" method + (e.g. directly from a view). + +Using the former pattern was straightforward since the form can guess from the +context (i.e. which method raised the exception) where the errors belong and +automatically process them. This remains the canonical way of adding errors +when possible. However the latter was fiddly and error-prone, since the burden +of handling edge cases fell on the user. + +The new :meth:`~django.forms.Form.add_error()` method allows adding errors +to specific form fields from anywhere without having to worry about the details +such as creating instances of ``django.forms.utils.ErrorList`` or dealing with +``Form.cleaned_data``. This new API replaces manipulating ``Form._errors`` +which now becomes a private API. + +See :ref:`validating-fields-with-clean` for an example using +``Form.add_error()``. + +Error metadata +^^^^^^^^^^^^^^ + +The :exc:`~django.core.exceptions.ValidationError` constructor accepts metadata +such as error ``code`` or ``params`` which are then available for interpolating +into the error message (see :ref:`raising-validation-error` for more details); +however, before Django 1.7 those metadata were discarded as soon as the errors +were added to :attr:`Form.errors `. + +:attr:`Form.errors ` and +``django.forms.utils.ErrorList`` now store the ``ValidationError`` instances +so these metadata can be retrieved at any time through the new +:meth:`Form.errors.as_data ` method. + +The retrieved ``ValidationError`` instances can then be identified thanks to +their error ``code`` which enables things like rewriting the error's message +or writing custom logic in a view when a given error is present. It can also +be used to serialize the errors in a custom format such as XML. + +The new :meth:`Form.errors.as_json() ` +method is a convenience method which returns error messages along with error +codes serialized as JSON. ``as_json()`` uses ``as_data()`` and gives an idea +of how the new system could be extended. + +Error containers and backward compatibility +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Heavy changes to the various error containers were necessary in order +to support the features above, specifically +:attr:`Form.errors `, +``django.forms.utils.ErrorList``, and the internal storages of +:exc:`~django.core.exceptions.ValidationError`. These containers which used +to store error strings now store ``ValidationError`` instances and public APIs +have been adapted to make this as transparent as possible, but if you've been +using private APIs, some of the changes are backwards incompatible; see +:ref:`validation-error-constructor-and-internal-storage` for more details. + Minor features ~~~~~~~~~~~~~~ @@ -562,15 +630,6 @@ Forms * It's now possible to remove a field from a ``Form`` when subclassing by setting the name to ``None``. -* The new :meth:`~django.forms.Form.add_error()` method allows adding errors - to specific form fields. - -* The dict-like attribute :attr:`~django.forms.Form.errors` now has two new - methods :meth:`~django.forms.Form.errors.as_data()` and - :meth:`~django.forms.Form.errors.as_json()`. The former returns a ``dict`` - that maps fields to their original errors, complete with all metadata - (error code and params), the latter returns the errors serialized as json. - * It's now possible to customize the error messages for ``ModelForm``’s ``unique``, ``unique_for_date``, and ``unique_together`` constraints. In order to support ``unique_together`` or any other ``NON_FIELD_ERROR``, @@ -1005,6 +1064,65 @@ This brings discovery of management commands in line with other parts of Django that rely on the order of :setting:`INSTALLED_APPS`, such as static files, templates, and translations. +.. _validation-error-constructor-and-internal-storage: + +``ValidationError`` constructor and internal storage +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The behavior of the ``ValidationError`` constructor has changed when it +receives a container of errors as an argument (e.g. a ``list`` or an +``ErrorList``): + +* It converts any strings it finds to instances of ``ValidationError`` + before adding them to its internal storage. + +* It doesn't store the given container but rather copies its content to its + own internal storage; previously the container itself was added to the + ``ValidationError`` instance and used as internal storage. + +This means that if you access the ``ValidationError`` internal storages, such +as ``error_list``; ``error_dict``; or the return value of +``update_error_dict()`` you may find instances of ``ValidationError`` where you +would have previously found strings. + +Also if you directly assigned the return value of ``update_error_dict()`` +to ``Form._errors`` you may inadvertently add `list` instances where +``ErrorList`` instances are expected. This is a problem because unlike a +simple `list`, an ``ErrorList`` knows how to handle instances of +``ValidationError``. + +Most use-cases that warranted using these private APIs are now covered by +the newly introduced :meth:`Form.add_error() ` +method:: + + # Old pattern: + try: + # ... + except ValidationError as e: + self._errors = e.update_error_dict(self._errors) + + # New pattern: + try: + # ... + except ValidationError as e: + self.add_error(None, e) + +If you need both Django <= 1.6 and 1.7 compatibility you can't use +:meth:`Form.add_error() ` since it +wasn't available before Django 1.7, but you can use the following +workaround to convert any ``list`` into ``ErrorList``:: + + try: + # ... + except ValidationError as e: + self._errors = e.update_error_dict(self._errors) + + # Additional code to ensure ``ErrorDict`` is exclusively + # composed of ``ErrorList`` instances. + for field, error_list in self._errors.items(): + if not isinstance(error_list, self.error_class): + self._errors[field] = self.error_class(error_list) + Behavior of ``LocMemCache`` regarding pickle errors ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/forms_tests/tests/test_forms.py b/tests/forms_tests/tests/test_forms.py index 985ff8e2e0..23de5024fb 100644 --- a/tests/forms_tests/tests/test_forms.py +++ b/tests/forms_tests/tests/test_forms.py @@ -740,6 +740,53 @@ class FormsTestCase(TestCase): with six.assertRaisesRegex(self, ValueError, "has no field named"): f.add_error('missing_field', 'Some error.') + def test_update_error_dict(self): + class CodeForm(Form): + code = CharField(max_length=10) + + def clean(self): + try: + raise ValidationError({'code': [ValidationError('Code error 1.')]}) + except ValidationError as e: + self._errors = e.update_error_dict(self._errors) + + try: + raise ValidationError({'code': [ValidationError('Code error 2.')]}) + except ValidationError as e: + self._errors = e.update_error_dict(self._errors) + + try: + raise ValidationError({'code': forms.ErrorList(['Code error 3.'])}) + except ValidationError as e: + self._errors = e.update_error_dict(self._errors) + + try: + raise ValidationError('Non-field error 1.') + except ValidationError as e: + self._errors = e.update_error_dict(self._errors) + + try: + raise ValidationError([ValidationError('Non-field error 2.')]) + except ValidationError as e: + self._errors = e.update_error_dict(self._errors) + + # Ensure that the newly added list of errors is an instance of ErrorList. + for field, error_list in self._errors.items(): + if not isinstance(error_list, self.error_class): + self._errors[field] = self.error_class(error_list) + + form = CodeForm({'code': 'hello'}) + # Trigger validation. + self.assertFalse(form.is_valid()) + + # Check that update_error_dict didn't lose track of the ErrorDict type. + self.assertTrue(isinstance(form._errors, forms.ErrorDict)) + + self.assertEqual(dict(form.errors), { + 'code': ['Code error 1.', 'Code error 2.', 'Code error 3.'], + NON_FIELD_ERRORS: ['Non-field error 1.', 'Non-field error 2.'], + }) + def test_has_error(self): class UserRegistration(Form): username = CharField(max_length=10)