From 38ba3775ec86718fabdf0ba1d5baf04a0f2164a7 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Thu, 18 Nov 2010 22:43:46 +0000 Subject: [PATCH] Fixed #14234 -- Re-validating a model instance added via ModelForm no longer throws spurious PK uniqueness errors. Thanks to David Reynolds and Jeremy Dunck. Also moved Model._adding to Model._state.adding to reduce instance namespace footprint. git-svn-id: http://code.djangoproject.com/svn/django/trunk@14612 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/db/models/base.py | 13 +++++++++--- django/forms/models.py | 4 ++-- tests/regressiontests/forms/models.py | 13 ++++++++++++ .../forms/tests/regressions.py | 21 +++++++++++++++++++ 4 files changed, 46 insertions(+), 5 deletions(-) diff --git a/django/db/models/base.py b/django/db/models/base.py index 796ccaaa5c..44f90a19f7 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -262,6 +262,10 @@ class ModelState(object): """ def __init__(self, db=None): self.db = db + # If true, uniqueness validation checks will consider this a new, as-yet-unsaved object. + # Necessary for correct validation of new instances of objects with explicit (non-auto) PKs. + # This impacts validation only; it has no effect on the actual save. + self.adding = False class Model(object): __metaclass__ = ModelBase @@ -555,12 +559,15 @@ class Model(object): # Store the database on which the object was saved self._state.db = using + # Once saved, this is no longer a to-be-added instance. + self._state.adding = False # Signal that the save is complete if origin and not meta.auto_created: signals.post_save.send(sender=origin, instance=self, created=(not record_exists), raw=raw, using=using) + save_base.alters_data = True def delete(self, using=None): @@ -701,7 +708,7 @@ class Model(object): if lookup_value is None: # no value, skip the lookup continue - if f.primary_key and not getattr(self, '_adding', False): + if f.primary_key and not self._state.adding: # no need to check for unique primary key when editing continue lookup_kwargs[str(field_name)] = lookup_value @@ -714,7 +721,7 @@ class Model(object): # Exclude the current object from the query if we are editing an # instance (as opposed to creating a new one) - if not getattr(self, '_adding', False) and self.pk is not None: + if not self._state.adding and self.pk is not None: qs = qs.exclude(pk=self.pk) if qs.exists(): @@ -744,7 +751,7 @@ class Model(object): qs = model_class._default_manager.filter(**lookup_kwargs) # Exclude the current object from the query if we are editing an # instance (as opposed to creating a new one) - if not getattr(self, '_adding', False) and self.pk is not None: + if not self._state.adding and self.pk is not None: qs = qs.exclude(pk=self.pk) if qs.exists(): diff --git a/django/forms/models.py b/django/forms/models.py index 2f41dbf9df..729f523103 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -254,10 +254,10 @@ class BaseModelForm(BaseForm): # if we didn't get an instance, instantiate a new one self.instance = opts.model() object_data = {} - self.instance._adding = True + self.instance._state.adding = True else: self.instance = instance - self.instance._adding = False + self.instance._state.adding = False object_data = model_to_dict(instance, opts.fields, opts.exclude) # if initial was provided, it should override the values from instance if initial is not None: diff --git a/tests/regressiontests/forms/models.py b/tests/regressiontests/forms/models.py index 0a0fea45ad..203980c11c 100644 --- a/tests/regressiontests/forms/models.py +++ b/tests/regressiontests/forms/models.py @@ -5,28 +5,34 @@ import tempfile from django.db import models from django.core.files.storage import FileSystemStorage + temp_storage_location = tempfile.mkdtemp() temp_storage = FileSystemStorage(location=temp_storage_location) + class BoundaryModel(models.Model): positive_integer = models.PositiveIntegerField(null=True, blank=True) + callable_default_value = 0 def callable_default(): global callable_default_value callable_default_value = callable_default_value + 1 return callable_default_value + class Defaults(models.Model): name = models.CharField(max_length=255, default='class default value') def_date = models.DateField(default = datetime.date(1980, 1, 1)) value = models.IntegerField(default=42) callable_default = models.IntegerField(default=callable_default) + class ChoiceModel(models.Model): """For ModelChoiceField and ModelMultipleChoiceField tests.""" name = models.CharField(max_length=10) + class ChoiceOptionModel(models.Model): """Destination for ChoiceFieldModel's ForeignKey. Can't reuse ChoiceModel because error_message tests require that it have no instances.""" @@ -38,6 +44,7 @@ class ChoiceOptionModel(models.Model): def __unicode__(self): return u'ChoiceOption %d' % self.pk + class ChoiceFieldModel(models.Model): """Model with ForeignKey to another model, for testing ModelForm generation with ModelChoiceField.""" @@ -51,11 +58,17 @@ class ChoiceFieldModel(models.Model): multi_choice_int = models.ManyToManyField(ChoiceOptionModel, blank=False, related_name='multi_choice_int', default=lambda: [1]) + class FileModel(models.Model): file = models.FileField(storage=temp_storage, upload_to='tests') + class Group(models.Model): name = models.CharField(max_length=10) def __unicode__(self): return u'%s' % self.name + + +class Cheese(models.Model): + name = models.CharField(max_length=100) diff --git a/tests/regressiontests/forms/tests/regressions.py b/tests/regressiontests/forms/tests/regressions.py index 3f69e0aa6e..6c6a308db5 100644 --- a/tests/regressiontests/forms/tests/regressions.py +++ b/tests/regressiontests/forms/tests/regressions.py @@ -3,6 +3,9 @@ from django.forms import * from django.utils.unittest import TestCase from django.utils.translation import ugettext_lazy, activate, deactivate +from regressiontests.forms.models import Cheese + + class FormsRegressionsTestCase(TestCase): def test_class(self): # Tests to prevent against recurrences of earlier bugs. @@ -120,3 +123,21 @@ class FormsRegressionsTestCase(TestCase): f = SomeForm({'field': ['