summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorViktor Danyliuk <v.v.danyliuk@gmail.com>2018-07-17 05:08:43 +0300
committerTim Graham <timograham@gmail.com>2018-07-16 22:08:43 -0400
commit6ae7aaa7d6eb880043c4c80009b36e2287bfcb92 (patch)
treead7628988f9ccf5f56dd181aa710cd15f82485b6
parent4d48ddd8f93800a80330ec1dee7b7d4afe6ea95a (diff)
Fixed #29413 -- Prevented evaluation of QuerySet.get_or_create()/update_or_create() defaults unless needed.
Removed the logic added in 81e05a418dc6f347d9627373288e773c477a9fe0 which was obsolete since dbffffa7dc95fc62cbecfd00284bde62ee796f48.
-rw-r--r--AUTHORS1
-rw-r--r--django/db/models/query.py23
-rw-r--r--tests/get_or_create/tests.py40
3 files changed, 42 insertions, 22 deletions
diff --git a/AUTHORS b/AUTHORS
index e5a22185a9..89b6540f69 100644
--- a/AUTHORS
+++ b/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>
diff --git a/django/db/models/query.py b/django/db/models/query.py
index c0d2dee3c1..e023d7749d 100644
--- a/django/db/models/query.py
+++ b/django/db/models/query.py
@@ -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):
"""
diff --git a/tests/get_or_create/tests.py b/tests/get_or_create/tests.py
index 1aed318608..e4647d2ab5 100644
--- a/tests/get_or_create/tests.py
+++ b/tests/get_or_create/tests.py
@@ -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'})