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

@@ -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():

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:

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)

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': ['<script>']})
         self.assertEqual(t.render(Context({'form': f})), u'<ul class="errorlist"><li>field<ul class="errorlist"><li>&quot;&lt;script&gt;&quot; is not a valid value for a primary key.</li></ul></li></ul>')
+
+    def test_regression_14234(self):
+        """
+        Re-cleaning an instance that was added via a ModelForm should not raise
+        a pk uniqueness error.
+
+        """
+        class CheeseForm(ModelForm):
+            class Meta:
+                model = Cheese
+
+        form = CheeseForm({
+            'name': 'Brie',
+        })
+        if form.is_valid():
+            obj = form.save()
+            obj.name = 'Camembert'
+            obj.full_clean()