From b68c7a5abbb737c54dff91e56e0e56e67cd3f718 Mon Sep 17 00:00:00 2001 From: Loic Bistuer Date: Tue, 1 Jul 2014 20:48:00 +0700 Subject: [PATCH] [1.7.x] 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. Backport of eb7df6b8d7 from master --- django/core/exceptions.py | 9 +- docs/releases/1.7.txt | 136 ++++++++++++++++++++++++-- tests/forms_tests/tests/test_forms.py | 48 +++++++++ 3 files changed, 178 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 a02bee3bcf..11270b3a27 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 a410e9a0f1..a2be118e10 100644 --- a/tests/forms_tests/tests/test_forms.py +++ b/tests/forms_tests/tests/test_forms.py @@ -6,6 +6,7 @@ import datetime import json import warnings +from django.core.exceptions import NON_FIELD_ERRORS from django.core.files.uploadedfile import SimpleUploadedFile from django.core.validators import RegexValidator from django.forms import ( @@ -739,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_dynamic_construction(self): # It's possible to construct a Form dynamically by adding to the self.fields # dictionary in __init__(). Don't forget to call Form.__init__() within the