From 5980b05c1fad69eef907e0076aa2dc837edab529 Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Fri, 24 Jul 2015 07:51:40 -0400 Subject: Fixed #25160 -- Moved unsaved model instance data loss check to Model.save() This mostly reverts 5643a3b51be338196d0b292d5626ad43648448d3 and 81e1a35c364e5353d2bf99368ad30a4184fbb653. Thanks Carl Meyer for review. --- tests/admin_utils/tests.py | 6 ++--- tests/contenttypes_tests/tests.py | 48 -------------------------------------- tests/generic_relations/tests.py | 10 ++++++++ tests/many_to_one/tests.py | 41 ++++++-------------------------- tests/model_fields/test_uuid.py | 17 ++++++++++++-- tests/one_to_one/tests.py | 49 +++++++++++---------------------------- 6 files changed, 48 insertions(+), 123 deletions(-) (limited to 'tests') diff --git a/tests/admin_utils/tests.py b/tests/admin_utils/tests.py index 18c95bf768..12254a983f 100644 --- a/tests/admin_utils/tests.py +++ b/tests/admin_utils/tests.py @@ -12,7 +12,7 @@ from django.contrib.admin.utils import ( label_for_field, lookup_field, quote, ) from django.db import DEFAULT_DB_ALIAS, models -from django.test import TestCase, override_settings +from django.test import SimpleTestCase, TestCase, override_settings from django.utils import six from django.utils.formats import localize from django.utils.safestring import mark_safe @@ -94,7 +94,7 @@ class NestedObjectsTests(TestCase): n.collect([Vehicle.objects.first()]) -class UtilsTests(TestCase): +class UtilsTests(SimpleTestCase): empty_value = '-empty-' @@ -115,7 +115,7 @@ class UtilsTests(TestCase): simple_function = lambda obj: SIMPLE_FUNCTION - site_obj = Site.objects.create(domain=SITE_NAME) + site_obj = Site(domain=SITE_NAME) article = Article( site=site_obj, title=TITLE_TEXT, diff --git a/tests/contenttypes_tests/tests.py b/tests/contenttypes_tests/tests.py index 3ff613d9c4..0d0f9d30b8 100644 --- a/tests/contenttypes_tests/tests.py +++ b/tests/contenttypes_tests/tests.py @@ -236,54 +236,6 @@ class GenericForeignKeyTests(IsolatedModelsTestCase): errors = checks.run_checks() self.assertEqual(errors, ['performed!']) - def test_unsaved_instance_on_generic_foreign_key(self): - """ - #10811 -- Assigning an unsaved object to GenericForeignKey - should raise an exception. - """ - class Model(models.Model): - content_type = models.ForeignKey(ContentType, models.SET_NULL, null=True) - object_id = models.PositiveIntegerField(null=True) - content_object = GenericForeignKey('content_type', 'object_id') - - author = Author(name='Author') - model = Model() - model.content_object = None # no error here as content_type allows None - with self.assertRaisesMessage(ValueError, - 'Cannot assign "%r": "%s" instance isn\'t saved in the database.' - % (author, author._meta.object_name)): - model.content_object = author # raised ValueError here as author is unsaved - - author.save() - model.content_object = author # no error because the instance is saved - - def test_unsaved_instance_on_generic_foreign_key_allowed_when_wanted(self): - """ - #24495 - Assigning an unsaved object to a GenericForeignKey - should be allowed when the allow_unsaved_instance_assignment - attribute has been set to True. - """ - class UnsavedGenericForeignKey(GenericForeignKey): - # A GenericForeignKey which can point to an unsaved object - allow_unsaved_instance_assignment = True - - class Band(models.Model): - name = models.CharField(max_length=50) - - class BandMember(models.Model): - band_ct = models.ForeignKey(ContentType, models.CASCADE) - band_id = models.PositiveIntegerField() - band = UnsavedGenericForeignKey('band_ct', 'band_id') - first_name = models.CharField(max_length=50) - last_name = models.CharField(max_length=50) - - beatles = Band(name='The Beatles') - john = BandMember(first_name='John', last_name='Lennon') - # This should not raise an exception as the GenericForeignKey between - # member and band has allow_unsaved_instance_assignment=True. - john.band = beatles - self.assertEqual(john.band, beatles) - class GenericRelationshipTests(IsolatedModelsTestCase): diff --git a/tests/generic_relations/tests.py b/tests/generic_relations/tests.py index 0162565969..c800bd77b0 100644 --- a/tests/generic_relations/tests.py +++ b/tests/generic_relations/tests.py @@ -4,6 +4,7 @@ from django import forms from django.contrib.contenttypes.forms import generic_inlineformset_factory from django.contrib.contenttypes.models import ContentType from django.core.exceptions import FieldError +from django.db import IntegrityError from django.db.models import Q from django.test import SimpleTestCase, TestCase from django.utils import six @@ -486,6 +487,15 @@ class GenericRelationsTests(TestCase): with self.assertRaisesMessage(FieldError, msg): TaggedItem.objects.get(content_object='') + def test_unsaved_instance_on_generic_foreign_key(self): + """ + Assigning an unsaved object to GenericForeignKey should raise an + exception on model.save(). + """ + quartz = Mineral(name="Quartz", hardness=7) + with self.assertRaises(IntegrityError): + TaggedItem.objects.create(tag="shiny", content_object=quartz) + class CustomWidget(forms.TextInput): pass diff --git a/tests/many_to_one/tests.py b/tests/many_to_one/tests.py index ba176c938c..610655beee 100644 --- a/tests/many_to_one/tests.py +++ b/tests/many_to_one/tests.py @@ -163,31 +163,6 @@ class ManyToOneTests(TestCase): self.assertFalse(hasattr(self.r2.article_set, 'remove')) self.assertFalse(hasattr(self.r2.article_set, 'clear')) - def test_assign_unsaved_check_override(self): - """ - #24495 - Assigning an unsaved object to a ForeignKey - should be allowed when the allow_unsaved_instance_assignment - attribute has been set to True. - """ - class UnsavedForeignKey(models.ForeignKey): - # A ForeignKey which can point to an unsaved object - allow_unsaved_instance_assignment = True - - class Band(models.Model): - name = models.CharField(max_length=50) - - class BandMember(models.Model): - band = UnsavedForeignKey(Band, models.CASCADE) - first_name = models.CharField(max_length=50) - last_name = models.CharField(max_length=50) - - beatles = Band(name='The Beatles') - john = BandMember(first_name='John', last_name='Lennon') - # This should not raise an exception as the ForeignKey between member - # and band has allow_unsaved_instance_assignment=True. - john.band = beatles - self.assertEqual(john.band, beatles) - def test_selects(self): self.r.article_set.create(headline="John's second story", pub_date=datetime.date(2005, 7, 29)) @@ -567,15 +542,13 @@ class ManyToOneTests(TestCase): # Creation using keyword argument and unsaved related instance (#8070). p = Parent() - with self.assertRaisesMessage(ValueError, - 'Cannot assign "%r": "%s" instance isn\'t saved in the database.' - % (p, Child.parent.field.remote_field.model._meta.object_name)): - Child(parent=p) - - with self.assertRaisesMessage(ValueError, - 'Cannot assign "%r": "%s" instance isn\'t saved in the database.' - % (p, Child.parent.field.remote_field.model._meta.object_name)): - ToFieldChild(parent=p) + msg = "save() prohibited to prevent data loss due to unsaved related object 'parent'." + with self.assertRaisesMessage(ValueError, msg): + Child.objects.create(parent=p) + + msg = "save() prohibited to prevent data loss due to unsaved related object 'parent'." + with self.assertRaisesMessage(ValueError, msg): + ToFieldChild.objects.create(parent=p) # Creation using attname keyword argument and an id will cause the # related object to be fetched. diff --git a/tests/model_fields/test_uuid.py b/tests/model_fields/test_uuid.py index bef1d54a74..343140c248 100644 --- a/tests/model_fields/test_uuid.py +++ b/tests/model_fields/test_uuid.py @@ -2,8 +2,10 @@ import json import uuid from django.core import exceptions, serializers -from django.db import models -from django.test import SimpleTestCase, TestCase +from django.db import IntegrityError, models +from django.test import ( + SimpleTestCase, TestCase, TransactionTestCase, skipUnlessDBFeature, +) from .models import ( NullableUUIDModel, PrimaryKeyUUIDModel, RelatedToUUIDModel, UUIDGrandchild, @@ -158,3 +160,14 @@ class TestAsPrimaryKey(TestCase): def test_two_level_foreign_keys(self): # exercises ForeignKey.get_db_prep_value() UUIDGrandchild().save() + + +class TestAsPrimaryKeyTransactionTests(TransactionTestCase): + # Need a TransactionTestCase to avoid deferring FK constraint checking. + available_apps = ['model_fields'] + + @skipUnlessDBFeature('supports_foreign_keys') + def test_unsaved_fk(self): + u1 = PrimaryKeyUUIDModel() + with self.assertRaises(IntegrityError): + RelatedToUUIDModel.objects.create(uuid_fk=u1) diff --git a/tests/one_to_one/tests.py b/tests/one_to_one/tests.py index 807d504998..ee0536906c 100644 --- a/tests/one_to_one/tests.py +++ b/tests/one_to_one/tests.py @@ -1,6 +1,6 @@ from __future__ import unicode_literals -from django.db import IntegrityError, connection, models, transaction +from django.db import IntegrityError, connection, transaction from django.test import TestCase from .models import ( @@ -134,41 +134,9 @@ class OneToOneTests(TestCase): should raise an exception. """ place = Place(name='User', address='London') - with self.assertRaisesMessage(ValueError, - 'Cannot assign "%r": "%s" instance isn\'t saved in the database.' - % (place, Restaurant.place.field.remote_field.model._meta.object_name)): + msg = "save() prohibited to prevent data loss due to unsaved related object 'place'." + with self.assertRaisesMessage(ValueError, msg): Restaurant.objects.create(place=place, serves_hot_dogs=True, serves_pizza=False) - bar = UndergroundBar() - p = Place(name='User', address='London') - with self.assertRaisesMessage(ValueError, - 'Cannot assign "%r": "%s" instance isn\'t saved in the database.' - % (bar, p._meta.object_name)): - p.undergroundbar = bar - - def test_unsaved_object_check_override(self): - """ - #24495 - Assigning an unsaved object to a OneToOneField - should be allowed when the allow_unsaved_instance_assignment - attribute has been set to True. - """ - class UnsavedOneToOneField(models.OneToOneField): - # A OneToOneField which can point to an unsaved object - allow_unsaved_instance_assignment = True - - class Band(models.Model): - name = models.CharField(max_length=50) - - class BandManager(models.Model): - band = UnsavedOneToOneField(Band, models.CASCADE) - first_name = models.CharField(max_length=50) - last_name = models.CharField(max_length=50) - - band = Band(name='The Beatles') - manager = BandManager(first_name='Brian', last_name='Epstein') - # This should not raise an exception as the OneToOneField between - # manager and band has allow_unsaved_instance_assignment=True. - manager.band = band - self.assertEqual(manager.band, band) def test_reverse_relationship_cache_cascade(self): """ @@ -249,6 +217,11 @@ class OneToOneTests(TestCase): r = Restaurant(place=p) self.assertIs(r.place, p) + # Creation using keyword argument and unsaved related instance (#8070). + p = Place() + r = Restaurant(place=p) + self.assertTrue(r.place is p) + # Creation using attname keyword argument and an id will cause the related # object to be fetched. p = Place.objects.get(name="Demon Dogs") @@ -392,8 +365,12 @@ class OneToOneTests(TestCase): """ p = Place() b = UndergroundBar.objects.create() + msg = ( + 'Cannot assign "": "Place" ' + 'instance isn\'t saved in the database.' + ) with self.assertNumQueries(0): - with self.assertRaises(ValueError): + with self.assertRaisesMessage(ValueError, msg): p.undergroundbar = b def test_nullable_o2o_delete(self): -- cgit v1.3