From d5fec03dad035e88494bf55194cbbbe4b20b5e5b Mon Sep 17 00:00:00 2001 From: Sarah Boyce <42296566+sarahboyce@users.noreply.github.com> Date: Fri, 13 Dec 2024 09:16:03 +0100 Subject: [PATCH] Refs #35060 -- Removed passing positional arguments to Model.save()/asave() per deprecation timeline. --- django/contrib/auth/base_user.py | 7 +- django/db/models/base.py | 70 +-------------- docs/ref/models/instances.txt | 4 - docs/releases/6.0.txt | 3 + tests/basic/tests.py | 141 +----------------------------- tests/update_only_fields/tests.py | 26 ------ 6 files changed, 8 insertions(+), 243 deletions(-) diff --git a/django/contrib/auth/base_user.py b/django/contrib/auth/base_user.py index 5bb88ac4dd..f19b2d89b9 100644 --- a/django/contrib/auth/base_user.py +++ b/django/contrib/auth/base_user.py @@ -58,11 +58,8 @@ class AbstractBaseUser(models.Model): def __str__(self): return self.get_username() - # RemovedInDjango60Warning: When the deprecation ends, replace with: - # def save(self, **kwargs): - # super().save(**kwargs) - def save(self, *args, **kwargs): - super().save(*args, **kwargs) + def save(self, **kwargs): + super().save(**kwargs) if self._password is not None: password_validation.password_changed(self._password, self) self._password = None diff --git a/django/db/models/base.py b/django/db/models/base.py index d7d207901b..575365e11c 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -50,7 +50,6 @@ from django.db.models.signals import ( pre_save, ) from django.db.models.utils import AltersData, make_model_tuple -from django.utils.deprecation import RemovedInDjango60Warning from django.utils.encoding import force_str from django.utils.hashable import make_hashable from django.utils.text import capfirst, get_text_list @@ -786,50 +785,9 @@ class Model(AltersData, metaclass=ModelBase): return getattr(self, field_name) return getattr(self, field.attname) - # RemovedInDjango60Warning: When the deprecation ends, remove completely. - def _parse_save_params(self, *args, method_name, **kwargs): - defaults = { - "force_insert": False, - "force_update": False, - "using": None, - "update_fields": None, - } - - warnings.warn( - f"Passing positional arguments to {method_name}() is deprecated", - RemovedInDjango60Warning, - stacklevel=3, - ) - total_len_args = len(args) + 1 # include self - max_len_args = len(defaults) + 1 - if total_len_args > max_len_args: - # Recreate the proper TypeError message from Python. - raise TypeError( - f"Model.{method_name}() takes from 1 to {max_len_args} positional " - f"arguments but {total_len_args} were given" - ) - - def get_param(param_name, param_value, arg_index): - if arg_index < len(args): - if param_value is not defaults[param_name]: - # Recreate the proper TypeError message from Python. - raise TypeError( - f"Model.{method_name}() got multiple values for argument " - f"'{param_name}'" - ) - return args[arg_index] - - return param_value - - return [get_param(k, v, i) for i, (k, v) in enumerate(kwargs.items())] - - # RemovedInDjango60Warning: When the deprecation ends, replace with: - # def save( - # self, *, force_insert=False, force_update=False, using=None, update_fields=None, - # ): def save( self, - *args, + *, force_insert=False, force_update=False, using=None, @@ -843,16 +801,6 @@ class Model(AltersData, metaclass=ModelBase): that the "save" must be an SQL insert or update (or equivalent for non-SQL backends), respectively. Normally, they should not be set. """ - # RemovedInDjango60Warning. - if args: - force_insert, force_update, using, update_fields = self._parse_save_params( - *args, - method_name="save", - force_insert=force_insert, - force_update=force_update, - using=using, - update_fields=update_fields, - ) self._prepare_related_fields_for_save(operation_name="save") @@ -908,28 +856,14 @@ class Model(AltersData, metaclass=ModelBase): save.alters_data = True - # RemovedInDjango60Warning: When the deprecation ends, replace with: - # async def asave( - # self, *, force_insert=False, force_update=False, using=None, update_fields=None, - # ): async def asave( self, - *args, + *, force_insert=False, force_update=False, using=None, update_fields=None, ): - # RemovedInDjango60Warning. - if args: - force_insert, force_update, using, update_fields = self._parse_save_params( - *args, - method_name="asave", - force_insert=force_insert, - force_update=force_update, - using=using, - update_fields=update_fields, - ) return await sync_to_async(self.save)( force_insert=force_insert, force_update=force_update, diff --git a/docs/ref/models/instances.txt b/docs/ref/models/instances.txt index c4bf8aae5f..be4ad4a4f4 100644 --- a/docs/ref/models/instances.txt +++ b/docs/ref/models/instances.txt @@ -430,10 +430,6 @@ method. See :ref:`overriding-model-methods` for more details. The model save process also has some subtleties; see the sections below. -.. deprecated:: 5.1 - - Support for positional arguments is deprecated. - Auto-incrementing primary keys ------------------------------ diff --git a/docs/releases/6.0.txt b/docs/releases/6.0.txt index a3a817045f..38b0681624 100644 --- a/docs/releases/6.0.txt +++ b/docs/releases/6.0.txt @@ -309,3 +309,6 @@ to remove usage of these features. * The ``django.contrib.gis.geoip2.GeoIP2.coords()`` method is removed. * The ``django.contrib.gis.geoip2.GeoIP2.open()`` method is removed. + +* Support for passing positional arguments to ``Model.save()`` and + ``Model.asave()`` is removed. diff --git a/tests/basic/tests.py b/tests/basic/tests.py index 6c2f9f2bd2..f6eabfaed7 100644 --- a/tests/basic/tests.py +++ b/tests/basic/tests.py @@ -20,9 +20,8 @@ from django.test import ( TransactionTestCase, skipUnlessDBFeature, ) -from django.test.utils import CaptureQueriesContext, ignore_warnings +from django.test.utils import CaptureQueriesContext from django.utils.connection import ConnectionDoesNotExist -from django.utils.deprecation import RemovedInDjango60Warning from django.utils.translation import gettext_lazy from .models import ( @@ -213,144 +212,6 @@ class ModelInstanceCreationTests(TestCase): with self.assertNumQueries(1): PrimaryKeyWithFalseyDbDefault().save() - def test_save_deprecation(self): - a = Article(headline="original", pub_date=datetime(2014, 5, 16)) - msg = "Passing positional arguments to save() is deprecated" - with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: - a.save(False, False, None, None) - self.assertEqual(Article.objects.count(), 1) - self.assertEqual(ctx.filename, __file__) - - def test_save_deprecation_positional_arguments_used(self): - a = Article() - fields = ["headline"] - with ( - self.assertWarns(RemovedInDjango60Warning), - mock.patch.object(a, "save_base") as mock_save_base, - ): - a.save(None, 1, 2, fields) - self.assertEqual( - mock_save_base.mock_calls, - [ - mock.call( - using=2, - force_insert=None, - force_update=1, - update_fields=frozenset(fields), - ) - ], - ) - - def test_save_too_many_positional_arguments(self): - a = Article() - msg = "Model.save() takes from 1 to 5 positional arguments but 6 were given" - with ( - self.assertWarns(RemovedInDjango60Warning), - self.assertRaisesMessage(TypeError, msg), - ): - a.save(False, False, None, None, None) - - def test_save_conflicting_positional_and_named_arguments(self): - a = Article() - cases = [ - ("force_insert", True, [42]), - ("force_update", None, [42, 41]), - ("using", "some-db", [42, 41, 40]), - ("update_fields", ["foo"], [42, 41, 40, 39]), - ] - for param_name, param_value, args in cases: - with self.subTest(param_name=param_name): - msg = f"Model.save() got multiple values for argument '{param_name}'" - with ( - self.assertWarns(RemovedInDjango60Warning), - self.assertRaisesMessage(TypeError, msg), - ): - a.save(*args, **{param_name: param_value}) - - async def test_asave_deprecation(self): - a = Article(headline="original", pub_date=datetime(2014, 5, 16)) - msg = "Passing positional arguments to asave() is deprecated" - with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: - await a.asave(False, False, None, None) - self.assertEqual(await Article.objects.acount(), 1) - self.assertEqual(ctx.filename, __file__) - - async def test_asave_deprecation_positional_arguments_used(self): - a = Article() - fields = ["headline"] - with ( - self.assertWarns(RemovedInDjango60Warning), - mock.patch.object(a, "save_base") as mock_save_base, - ): - await a.asave(None, 1, 2, fields) - self.assertEqual( - mock_save_base.mock_calls, - [ - mock.call( - using=2, - force_insert=None, - force_update=1, - update_fields=frozenset(fields), - ) - ], - ) - - async def test_asave_too_many_positional_arguments(self): - a = Article() - msg = "Model.asave() takes from 1 to 5 positional arguments but 6 were given" - with ( - self.assertWarns(RemovedInDjango60Warning), - self.assertRaisesMessage(TypeError, msg), - ): - await a.asave(False, False, None, None, None) - - async def test_asave_conflicting_positional_and_named_arguments(self): - a = Article() - cases = [ - ("force_insert", True, [42]), - ("force_update", None, [42, 41]), - ("using", "some-db", [42, 41, 40]), - ("update_fields", ["foo"], [42, 41, 40, 39]), - ] - for param_name, param_value, args in cases: - with self.subTest(param_name=param_name): - msg = f"Model.asave() got multiple values for argument '{param_name}'" - with ( - self.assertWarns(RemovedInDjango60Warning), - self.assertRaisesMessage(TypeError, msg), - ): - await a.asave(*args, **{param_name: param_value}) - - @ignore_warnings(category=RemovedInDjango60Warning) - def test_save_positional_arguments(self): - a = Article.objects.create(headline="original", pub_date=datetime(2014, 5, 16)) - a.headline = "changed" - - a.save(False, False, None, ["pub_date"]) - a.refresh_from_db() - self.assertEqual(a.headline, "original") - - a.headline = "changed" - a.save(False, False, None, ["pub_date", "headline"]) - a.refresh_from_db() - self.assertEqual(a.headline, "changed") - - @ignore_warnings(category=RemovedInDjango60Warning) - async def test_asave_positional_arguments(self): - a = await Article.objects.acreate( - headline="original", pub_date=datetime(2014, 5, 16) - ) - a.headline = "changed" - - await a.asave(False, False, None, ["pub_date"]) - await a.arefresh_from_db() - self.assertEqual(a.headline, "original") - - a.headline = "changed" - await a.asave(False, False, None, ["pub_date", "headline"]) - await a.arefresh_from_db() - self.assertEqual(a.headline, "changed") - class ModelTest(TestCase): def test_objects_attribute_is_only_available_on_the_class_itself(self): diff --git a/tests/update_only_fields/tests.py b/tests/update_only_fields/tests.py index 43f3e1fd16..2c86995799 100644 --- a/tests/update_only_fields/tests.py +++ b/tests/update_only_fields/tests.py @@ -1,6 +1,5 @@ from django.db.models.signals import post_save, pre_save from django.test import TestCase -from django.utils.deprecation import RemovedInDjango60Warning from .models import Account, Employee, Person, Profile, ProxyEmployee @@ -257,31 +256,6 @@ class UpdateOnlyFieldsTests(TestCase): pre_save.disconnect(pre_save_receiver) post_save.disconnect(post_save_receiver) - def test_empty_update_fields_positional_save(self): - s = Person.objects.create(name="Sara", gender="F") - - msg = "Passing positional arguments to save() is deprecated" - with ( - self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx, - self.assertNumQueries(0), - ): - s.save(False, False, None, []) - self.assertEqual(ctx.filename, __file__) - - async def test_empty_update_fields_positional_asave(self): - s = await Person.objects.acreate(name="Sara", gender="F") - # Workaround for a lack of async assertNumQueries. - s.name = "Other" - - msg = "Passing positional arguments to asave() is deprecated" - with self.assertWarnsMessage(RemovedInDjango60Warning, msg) as ctx: - await s.asave(False, False, None, []) - self.assertEqual(ctx.filename, __file__) - - # No save occurred for an empty update_fields. - await s.arefresh_from_db() - self.assertEqual(s.name, "Sara") - def test_num_queries_inheritance(self): s = Employee.objects.create(name="Sara", gender="F") s.employee_num = 1