Fixed #29413 -- Prevented evaluation of QuerySet.get_or_create()/update_or_create() defaults unless needed.
Removed the logic added in81e05a418d
which was obsolete sincedbffffa7dc
.
This commit is contained in:
parent
4d48ddd8f9
commit
6ae7aaa7d6
1
AUTHORS
1
AUTHORS
|
@ -849,6 +849,7 @@ answer newbie questions, and generally made Django that much better:
|
|||
Vasil Vangelovski
|
||||
Victor Andrée
|
||||
viestards.lists@gmail.com
|
||||
Viktor Danyliuk <v.v.danyliuk@gmail.com>
|
||||
Ville Säävuori <http://www.unessa.net/>
|
||||
Vinay Sajip <vinay_sajip@yahoo.co.uk>
|
||||
Vincent Foley <vfoleybourgon@yahoo.ca>
|
||||
|
|
|
@ -478,14 +478,14 @@ class QuerySet:
|
|||
Return a tuple of (object, created), where created is a boolean
|
||||
specifying whether an object was created.
|
||||
"""
|
||||
lookup, params = self._extract_model_params(defaults, **kwargs)
|
||||
# The get() needs to be targeted at the write database in order
|
||||
# to avoid potential transaction consistency problems.
|
||||
self._for_write = True
|
||||
try:
|
||||
return self.get(**lookup), False
|
||||
return self.get(**kwargs), False
|
||||
except self.model.DoesNotExist:
|
||||
return self._create_object_from_params(lookup, params)
|
||||
params = self._extract_model_params(defaults, **kwargs)
|
||||
return self._create_object_from_params(kwargs, params)
|
||||
|
||||
def update_or_create(self, defaults=None, **kwargs):
|
||||
"""
|
||||
|
@ -495,13 +495,13 @@ class QuerySet:
|
|||
specifying whether an object was created.
|
||||
"""
|
||||
defaults = defaults or {}
|
||||
lookup, params = self._extract_model_params(defaults, **kwargs)
|
||||
self._for_write = True
|
||||
with transaction.atomic(using=self.db):
|
||||
try:
|
||||
obj = self.select_for_update().get(**lookup)
|
||||
obj = self.select_for_update().get(**kwargs)
|
||||
except self.model.DoesNotExist:
|
||||
obj, created = self._create_object_from_params(lookup, params)
|
||||
params = self._extract_model_params(defaults, **kwargs)
|
||||
obj, created = self._create_object_from_params(kwargs, params)
|
||||
if created:
|
||||
return obj, created
|
||||
for k, v in defaults.items():
|
||||
|
@ -528,15 +528,10 @@ class QuerySet:
|
|||
|
||||
def _extract_model_params(self, defaults, **kwargs):
|
||||
"""
|
||||
Prepare `lookup` (kwargs that are valid model attributes), `params`
|
||||
(for creating a model instance) based on given kwargs; for use by
|
||||
get_or_create() and update_or_create().
|
||||
Prepare `params` for creating a model instance based on the given
|
||||
kwargs; for use by get_or_create() and update_or_create().
|
||||
"""
|
||||
defaults = defaults or {}
|
||||
lookup = kwargs.copy()
|
||||
for f in self.model._meta.fields:
|
||||
if f.attname in lookup:
|
||||
lookup[f.name] = lookup.pop(f.attname)
|
||||
params = {k: v for k, v in kwargs.items() if LOOKUP_SEP not in k}
|
||||
params.update(defaults)
|
||||
property_names = self.model._meta._property_names
|
||||
|
@ -554,7 +549,7 @@ class QuerySet:
|
|||
self.model._meta.object_name,
|
||||
"', '".join(sorted(invalid_params)),
|
||||
))
|
||||
return lookup, params
|
||||
return params
|
||||
|
||||
def _earliest_or_latest(self, *fields, field_name=None):
|
||||
"""
|
||||
|
|
|
@ -5,9 +5,8 @@ from threading import Thread
|
|||
|
||||
from django.core.exceptions import FieldError
|
||||
from django.db import DatabaseError, IntegrityError, connection
|
||||
from django.test import (
|
||||
SimpleTestCase, TestCase, TransactionTestCase, skipUnlessDBFeature,
|
||||
)
|
||||
from django.test import TestCase, TransactionTestCase, skipUnlessDBFeature
|
||||
from django.utils.functional import lazy
|
||||
|
||||
from .models import (
|
||||
Author, Book, DefaultPerson, ManualPrimaryKeyTest, Person, Profile,
|
||||
|
@ -178,6 +177,15 @@ class GetOrCreateTests(TestCase):
|
|||
defaults={"birthday": lambda: raise_exception()},
|
||||
)
|
||||
|
||||
def test_defaults_not_evaluated_unless_needed(self):
|
||||
"""`defaults` aren't evaluated if the instance isn't created."""
|
||||
def raise_exception():
|
||||
raise AssertionError
|
||||
obj, created = Person.objects.get_or_create(
|
||||
first_name='John', defaults=lazy(raise_exception, object)(),
|
||||
)
|
||||
self.assertFalse(created)
|
||||
|
||||
|
||||
class GetOrCreateTestsWithManualPKs(TestCase):
|
||||
|
||||
|
@ -443,6 +451,19 @@ class UpdateOrCreateTests(TestCase):
|
|||
self.assertIs(created, False)
|
||||
self.assertEqual(obj.last_name, 'NotHarrison')
|
||||
|
||||
def test_defaults_not_evaluated_unless_needed(self):
|
||||
"""`defaults` aren't evaluated if the instance isn't created."""
|
||||
Person.objects.create(
|
||||
first_name='John', last_name='Lennon', birthday=date(1940, 10, 9)
|
||||
)
|
||||
|
||||
def raise_exception():
|
||||
raise AssertionError
|
||||
obj, created = Person.objects.get_or_create(
|
||||
first_name='John', defaults=lazy(raise_exception, object)(),
|
||||
)
|
||||
self.assertFalse(created)
|
||||
|
||||
|
||||
class UpdateOrCreateTestsWithManualPKs(TestCase):
|
||||
|
||||
|
@ -515,15 +536,17 @@ class UpdateOrCreateTransactionTests(TransactionTestCase):
|
|||
self.assertEqual(updated_person.last_name, 'NotLennon')
|
||||
|
||||
|
||||
class InvalidCreateArgumentsTests(SimpleTestCase):
|
||||
class InvalidCreateArgumentsTests(TransactionTestCase):
|
||||
available_apps = ['get_or_create']
|
||||
msg = "Invalid field name(s) for model Thing: 'nonexistent'."
|
||||
bad_field_msg = "Cannot resolve keyword 'nonexistent' into field. Choices are: id, name, tags"
|
||||
|
||||
def test_get_or_create_with_invalid_defaults(self):
|
||||
with self.assertRaisesMessage(FieldError, self.msg):
|
||||
Thing.objects.get_or_create(name='a', defaults={'nonexistent': 'b'})
|
||||
|
||||
def test_get_or_create_with_invalid_kwargs(self):
|
||||
with self.assertRaisesMessage(FieldError, self.msg):
|
||||
with self.assertRaisesMessage(FieldError, self.bad_field_msg):
|
||||
Thing.objects.get_or_create(name='a', nonexistent='b')
|
||||
|
||||
def test_update_or_create_with_invalid_defaults(self):
|
||||
|
@ -531,11 +554,11 @@ class InvalidCreateArgumentsTests(SimpleTestCase):
|
|||
Thing.objects.update_or_create(name='a', defaults={'nonexistent': 'b'})
|
||||
|
||||
def test_update_or_create_with_invalid_kwargs(self):
|
||||
with self.assertRaisesMessage(FieldError, self.msg):
|
||||
with self.assertRaisesMessage(FieldError, self.bad_field_msg):
|
||||
Thing.objects.update_or_create(name='a', nonexistent='b')
|
||||
|
||||
def test_multiple_invalid_fields(self):
|
||||
with self.assertRaisesMessage(FieldError, "Invalid field name(s) for model Thing: 'invalid', 'nonexistent'"):
|
||||
with self.assertRaisesMessage(FieldError, self.bad_field_msg):
|
||||
Thing.objects.update_or_create(name='a', nonexistent='b', defaults={'invalid': 'c'})
|
||||
|
||||
def test_property_attribute_without_setter_defaults(self):
|
||||
|
@ -543,5 +566,6 @@ class InvalidCreateArgumentsTests(SimpleTestCase):
|
|||
Thing.objects.update_or_create(name='a', defaults={'name_in_all_caps': 'FRANK'})
|
||||
|
||||
def test_property_attribute_without_setter_kwargs(self):
|
||||
with self.assertRaisesMessage(FieldError, "Invalid field name(s) for model Thing: 'name_in_all_caps'"):
|
||||
msg = "Cannot resolve keyword 'name_in_all_caps' into field. Choices are: id, name, tags"
|
||||
with self.assertRaisesMessage(FieldError, msg):
|
||||
Thing.objects.update_or_create(name_in_all_caps='FRANK', defaults={'name': 'Frank'})
|
||||
|
|
Loading…
Reference in New Issue