mirror of
https://github.com/django/django.git
synced 2025-03-12 18:30:48 +00:00
Fixed #33579 -- Specialized exception raised on forced update failures.
Raising DatabaseError directly made it harder than it should to differentiate between IntegrityError when a forced update resulted in no affected rows. Introducing a specialized exception allows for callers to more easily silence, log, or turn them update failures into user facing exceptions (e.g. 404s). Thanks Mariusz for the review.
This commit is contained in:
parent
c1257350ca
commit
ab148c02ce
@ -25,6 +25,10 @@ class ObjectDoesNotExist(Exception):
|
|||||||
silent_variable_failure = True
|
silent_variable_failure = True
|
||||||
|
|
||||||
|
|
||||||
|
class ObjectNotUpdated(Exception):
|
||||||
|
"""The updated object no longer exists."""
|
||||||
|
|
||||||
|
|
||||||
class MultipleObjectsReturned(Exception):
|
class MultipleObjectsReturned(Exception):
|
||||||
"""The query returned multiple objects when only one was expected."""
|
"""The query returned multiple objects when only one was expected."""
|
||||||
|
|
||||||
|
@ -17,6 +17,7 @@ from django.core.exceptions import (
|
|||||||
FieldError,
|
FieldError,
|
||||||
MultipleObjectsReturned,
|
MultipleObjectsReturned,
|
||||||
ObjectDoesNotExist,
|
ObjectDoesNotExist,
|
||||||
|
ObjectNotUpdated,
|
||||||
ValidationError,
|
ValidationError,
|
||||||
)
|
)
|
||||||
from django.db import (
|
from django.db import (
|
||||||
@ -171,6 +172,23 @@ class ModelBase(type):
|
|||||||
attached_to=new_class,
|
attached_to=new_class,
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
new_class.add_to_class(
|
||||||
|
"NotUpdated",
|
||||||
|
subclass_exception(
|
||||||
|
"NotUpdated",
|
||||||
|
tuple(
|
||||||
|
x.NotUpdated
|
||||||
|
for x in parents
|
||||||
|
if hasattr(x, "_meta") and not x._meta.abstract
|
||||||
|
)
|
||||||
|
# Subclass DatabaseError as well for backward compatibility
|
||||||
|
# reasons as __subclasshook__ is not taken into account on
|
||||||
|
# exception handling.
|
||||||
|
or (ObjectNotUpdated, DatabaseError),
|
||||||
|
module,
|
||||||
|
attached_to=new_class,
|
||||||
|
),
|
||||||
|
)
|
||||||
if base_meta and not base_meta.abstract:
|
if base_meta and not base_meta.abstract:
|
||||||
# Non-abstract child classes inherit some attributes from their
|
# Non-abstract child classes inherit some attributes from their
|
||||||
# non-abstract parent (unless an ABC comes before it in the
|
# non-abstract parent (unless an ABC comes before it in the
|
||||||
@ -1073,9 +1091,11 @@ class Model(AltersData, metaclass=ModelBase):
|
|||||||
base_qs, using, pk_val, values, update_fields, forced_update
|
base_qs, using, pk_val, values, update_fields, forced_update
|
||||||
)
|
)
|
||||||
if force_update and not updated:
|
if force_update and not updated:
|
||||||
raise DatabaseError("Forced update did not affect any rows.")
|
raise self.NotUpdated("Forced update did not affect any rows.")
|
||||||
if update_fields and not updated:
|
if update_fields and not updated:
|
||||||
raise DatabaseError("Save with update_fields did not affect any rows.")
|
raise self.NotUpdated(
|
||||||
|
"Save with update_fields did not affect any rows."
|
||||||
|
)
|
||||||
if not updated:
|
if not updated:
|
||||||
if meta.order_with_respect_to:
|
if meta.order_with_respect_to:
|
||||||
# If this is a model with an order_with_respect_to
|
# If this is a model with an order_with_respect_to
|
||||||
|
@ -33,6 +33,20 @@ Django core exception classes are defined in ``django.core.exceptions``.
|
|||||||
|
|
||||||
See :meth:`~django.db.models.query.QuerySet.get()`.
|
See :meth:`~django.db.models.query.QuerySet.get()`.
|
||||||
|
|
||||||
|
``ObjectNotUpdated``
|
||||||
|
--------------------
|
||||||
|
|
||||||
|
.. versionadded:: 6.0
|
||||||
|
|
||||||
|
.. exception:: ObjectNotUpdated
|
||||||
|
|
||||||
|
The base class for :exc:`Model.NotUpdated
|
||||||
|
<django.db.models.Model.NotUpdated>` exceptions. A ``try/except`` for
|
||||||
|
``ObjectNotUpdated`` will catch
|
||||||
|
:exc:`~django.db.models.Model.NotUpdated` exceptions for all models.
|
||||||
|
|
||||||
|
See :meth:`~django.db.models.Model.save()`.
|
||||||
|
|
||||||
``EmptyResultSet``
|
``EmptyResultSet``
|
||||||
------------------
|
------------------
|
||||||
|
|
||||||
|
@ -39,6 +39,23 @@ Attributes
|
|||||||
The exception is a subclass of
|
The exception is a subclass of
|
||||||
:exc:`django.core.exceptions.MultipleObjectsReturned`.
|
:exc:`django.core.exceptions.MultipleObjectsReturned`.
|
||||||
|
|
||||||
|
``NotUpdated``
|
||||||
|
--------------
|
||||||
|
|
||||||
|
.. versionadded:: 6.0
|
||||||
|
|
||||||
|
.. exception:: Model.NotUpdated
|
||||||
|
|
||||||
|
This exception is raised when :ref:`a forced update
|
||||||
|
<ref-models-force-insert>` of a :class:`~django.db.models.Model` instance
|
||||||
|
does not affect any rows.
|
||||||
|
|
||||||
|
Django provides a ``NotUpdated`` exception as an attribute of each model
|
||||||
|
class to identify the class of object that could not be updated, allowing
|
||||||
|
you to catch exceptions for a particular model class. The exception is a
|
||||||
|
subclass of :exc:`django.core.exceptions.ObjectNotUpdated` and inherits
|
||||||
|
from :exc:`django.db.DatabaseError` for backward compatibility reasons.
|
||||||
|
|
||||||
``objects``
|
``objects``
|
||||||
-----------
|
-----------
|
||||||
|
|
||||||
|
@ -601,6 +601,12 @@ You can pass ``force_insert=(models.Model,)`` to force an ``INSERT`` statement
|
|||||||
for all parents. By default, ``force_insert=True`` only forces the insertion of
|
for all parents. By default, ``force_insert=True`` only forces the insertion of
|
||||||
a new row for the current model.
|
a new row for the current model.
|
||||||
|
|
||||||
|
.. versionchanged:: 6.0
|
||||||
|
|
||||||
|
When a forced update does not affect any rows a
|
||||||
|
:exc:`~django.db.models.Model.NotUpdated` exception is raised. On previous
|
||||||
|
versions a generic :exc:`django.db.DatabaseError` was raised.
|
||||||
|
|
||||||
It should be very rare that you'll need to use these parameters. Django will
|
It should be very rare that you'll need to use these parameters. Django will
|
||||||
almost always do the right thing and trying to override that will lead to
|
almost always do the right thing and trying to override that will lead to
|
||||||
errors that are difficult to track down. This feature is for advanced use
|
errors that are difficult to track down. This feature is for advanced use
|
||||||
|
@ -194,6 +194,11 @@ Models
|
|||||||
values concatenated into a string, separated by the ``delimiter`` string.
|
values concatenated into a string, separated by the ``delimiter`` string.
|
||||||
This aggregate was previously supported only for PostgreSQL.
|
This aggregate was previously supported only for PostgreSQL.
|
||||||
|
|
||||||
|
* The :meth:`~django.db.models.Model.save` method now raises a specialized
|
||||||
|
:exc:`Model.NotUpdated <django.db.models.Model.NotUpdated>` exception, when
|
||||||
|
:ref:`a forced update <ref-models-force-insert>` results in no affected rows,
|
||||||
|
instead of a generic :exc:`django.db.DatabaseError`.
|
||||||
|
|
||||||
Requests and Responses
|
Requests and Responses
|
||||||
~~~~~~~~~~~~~~~~~~~~~~
|
~~~~~~~~~~~~~~~~~~~~~~
|
||||||
|
|
||||||
|
@ -1,3 +1,4 @@
|
|||||||
|
from django.core.exceptions import ObjectNotUpdated
|
||||||
from django.db import DatabaseError, IntegrityError, models, transaction
|
from django.db import DatabaseError, IntegrityError, models, transaction
|
||||||
from django.test import TestCase
|
from django.test import TestCase
|
||||||
|
|
||||||
@ -50,8 +51,14 @@ class ForceTests(TestCase):
|
|||||||
# the data isn't in the database already.
|
# the data isn't in the database already.
|
||||||
obj = WithCustomPK(name=1, value=1)
|
obj = WithCustomPK(name=1, value=1)
|
||||||
msg = "Forced update did not affect any rows."
|
msg = "Forced update did not affect any rows."
|
||||||
with self.assertRaisesMessage(DatabaseError, msg):
|
# Make sure backward compatibility with DatabaseError is preserved.
|
||||||
with transaction.atomic():
|
exceptions = [DatabaseError, ObjectNotUpdated, WithCustomPK.NotUpdated]
|
||||||
|
for exception in exceptions:
|
||||||
|
with (
|
||||||
|
self.subTest(exception),
|
||||||
|
self.assertRaisesMessage(DatabaseError, msg),
|
||||||
|
transaction.atomic(),
|
||||||
|
):
|
||||||
obj.save(force_update=True)
|
obj.save(force_update=True)
|
||||||
|
|
||||||
|
|
||||||
|
@ -1,3 +1,5 @@
|
|||||||
|
from django.core.exceptions import ObjectNotUpdated
|
||||||
|
from django.db import DatabaseError, transaction
|
||||||
from django.db.models.signals import post_save, pre_save
|
from django.db.models.signals import post_save, pre_save
|
||||||
from django.test import TestCase
|
from django.test import TestCase
|
||||||
|
|
||||||
@ -292,3 +294,17 @@ class UpdateOnlyFieldsTests(TestCase):
|
|||||||
employee_boss = Employee.objects.create(name="Boss", gender="F")
|
employee_boss = Employee.objects.create(name="Boss", gender="F")
|
||||||
with self.assertRaisesMessage(ValueError, self.msg % "id"):
|
with self.assertRaisesMessage(ValueError, self.msg % "id"):
|
||||||
employee_boss.save(update_fields=["id"])
|
employee_boss.save(update_fields=["id"])
|
||||||
|
|
||||||
|
def test_update_fields_not_updated(self):
|
||||||
|
obj = Person.objects.create(name="Sara", gender="F")
|
||||||
|
Person.objects.filter(pk=obj.pk).delete()
|
||||||
|
msg = "Save with update_fields did not affect any rows."
|
||||||
|
# Make sure backward compatibility with DatabaseError is preserved.
|
||||||
|
exceptions = [DatabaseError, ObjectNotUpdated, Person.NotUpdated]
|
||||||
|
for exception in exceptions:
|
||||||
|
with (
|
||||||
|
self.subTest(exception),
|
||||||
|
self.assertRaisesMessage(DatabaseError, msg),
|
||||||
|
transaction.atomic(),
|
||||||
|
):
|
||||||
|
obj.save(update_fields=["name"])
|
||||||
|
Loading…
x
Reference in New Issue
Block a user