Ticket #17341: 17341.2.diff

django/db/models/base.py

@@ -465 +465 @@
 
     save.alters_data = True
 
-    def save_base(self, raw=False, cls=None, origin=None, force_insert=False,
-            force_update=False, using=None):
+    def save_base(self, raw=False, cls=None, topmost_concrete=True,
+                  force_insert=False, force_update=False, using=None):
         """
         Does the heavy-lifting involved in saving. Subclasses shouldn't need to
         override this method. It's separate from save() in order to hide the
         need for overrides of save() to pass around internal-only parameters
-        ('raw', 'cls', and 'origin').
+        ('raw', 'topmost_concrete', and 'origin').
+
+        Internal parameters:
+         - raw: If set, save just the table used by self.__class__. That is, do
+           not follow inheritance chains, except for proxy-skipping.
+         - topmost_concrete: Kept True as long as we are skipping the outmost
+           proxy models. For parents of a concrete model (that is multitable
+           inherited model's parents), this will be False.
+         - cls: This is the current class under save. In inheritance chains this
+           is given from the previous save_base, but in the topmost save this
+           will be given in as None.
         """
         using = using or router.db_for_write(self.__class__, instance=self)
         assert not (force_insert and force_update)
+
         if cls is None:
             cls = self.__class__
-            meta = cls._meta
-            if not meta.proxy:
-                origin = cls
-        else:
-            meta = cls._meta
+        meta = cls._meta
+
+        # Skip proxy chains. Defer the save of proxy objects to their actual
+        # underlying model.
+        if meta.proxy:
+            parent, field = meta.parents.items()[0]
+            self.save_base(cls=parent, topmost_concrete=topmost_concrete, using=using)
+            return
 
-        if origin and not meta.auto_created:
-            signals.pre_save.send(sender=origin, instance=self, raw=raw, using=using)
+        if topmost_concrete and not meta.auto_created:
+            signals.pre_save.send(sender=self.__class__, instance=self,
+                                  raw=raw, using=using)
 
         # If we are in a raw save, save the object exactly as presented.
         # That means that we don't try to be smart about saving attributes
         # that might have come from the parent class - we just save the
         # attributes we have been given to the class we have been given.
-        # We also go through this process to defer the save of proxy objects
-        # to their actual underlying model.
-        if not raw or meta.proxy:
-            if meta.proxy:
-                org = cls
-            else:
-                org = None
+        if not raw:
             for parent, field in meta.parents.items():
                 # At this point, parent's primary key field may be unknown
                 # (for example, from administration form which doesn't fill
                 # this field). If so, fill it.
-                if field and getattr(self, parent._meta.pk.attname) is None and getattr(self, field.attname) is not None:
+                if (field and getattr(self, parent._meta.pk.attname) is None
+                        and getattr(self, field.attname) is not None):
                     setattr(self, parent._meta.pk.attname, getattr(self, field.attname))
 
-                self.save_base(cls=parent, origin=org, using=using)
-
+                self.save_base(cls=parent, topmost_concrete=False, using=using)
                 if field:
                     setattr(self, field.attname, self._get_pk_val(parent._meta))
-            if meta.proxy:
-                return
-
-        if not meta.proxy:
-            non_pks = [f for f in meta.local_fields if not f.primary_key]
-
-            # First, try an UPDATE. If that doesn't update anything, do an INSERT.
-            pk_val = self._get_pk_val(meta)
-            pk_set = pk_val is not None
-            record_exists = True
-            manager = cls._base_manager
-            if pk_set:
-                # Determine whether a record with the primary key already exists.
-                if (force_update or (not force_insert and
-                        manager.using(using).filter(pk=pk_val).exists())):
-                    # It does already exist, so do an UPDATE.
-                    if force_update or non_pks:
-                        values = [(f, None, (raw and getattr(self, f.attname) or f.pre_save(self, False))) for f in non_pks]
-                        if values:
-                            rows = manager.using(using).filter(pk=pk_val)._update(values)
-                            if force_update and not rows:
-                                raise DatabaseError("Forced update did not affect any rows.")
-                else:
-                    record_exists = False
-            if not pk_set or not record_exists:
-                if meta.order_with_respect_to:
-                    # If this is a model with an order_with_respect_to
-                    # autopopulate the _order field
-                    field = meta.order_with_respect_to
-                    order_value = manager.using(using).filter(**{field.name: getattr(self, field.attname)}).count()
-                    self._order = order_value
-
-                fields = meta.local_fields
-                if not pk_set:
-                    if force_update:
-                        raise ValueError("Cannot force an update in save() with no primary key.")
-                    fields = [f for f in fields if not isinstance(f, AutoField)]
 
+        non_pks = [f for f in meta.local_fields if not f.primary_key]
+
+        # First, try an UPDATE. If that doesn't update anything, do an INSERT.
+        pk_val = self._get_pk_val(meta)
+        pk_set = pk_val is not None
+        record_exists = True
+        manager = cls._base_manager
+        if pk_set:
+            # Determine whether a record with the primary key already exists.
+            if (force_update or (not force_insert and
+                    manager.using(using).filter(pk=pk_val).exists())):
+                # It does already exist, so do an UPDATE.
+                if force_update or non_pks:
+                    values = [(f, None, (raw and getattr(self, f.attname) or f.pre_save(self, False))) for f in non_pks]
+                    if values:
+                        rows = manager.using(using).filter(pk=pk_val)._update(values)
+                        if force_update and not rows:
+                            raise DatabaseError("Forced update did not affect any rows.")
+            else:
                 record_exists = False
+        if not pk_set or not record_exists:
+            if meta.order_with_respect_to:
+                # If this is a model with an order_with_respect_to
+                # autopopulate the _order field
+                field = meta.order_with_respect_to
+                order_value = manager.using(using).filter(**{field.name: getattr(self, field.attname)}).count()
+                self._order = order_value
 
-                update_pk = bool(meta.has_auto_field and not pk_set)
-                result = manager._insert([self], fields=fields, return_id=update_pk, using=using, raw=raw)
+            fields = meta.local_fields
+            if not pk_set:
+                if force_update:
+                    raise ValueError("Cannot force an update in save() with no primary key.")
+                fields = [f for f in fields if not isinstance(f, AutoField)]
 
-                if update_pk:
-                    setattr(self, meta.pk.attname, result)
-            transaction.commit_unless_managed(using=using)
+            record_exists = False
+
+            update_pk = bool(meta.has_auto_field and not pk_set)
+            result = manager._insert([self], fields=fields, return_id=update_pk, using=using, raw=raw)
+
+            if update_pk:
+                setattr(self, meta.pk.attname, result)
 
         # 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)
+        # Commit the transaction & signal that the save is complete.
+        if topmost_concrete:
+            transaction.commit_unless_managed(using=using)
+            if not meta.auto_created:
+                signals.post_save.send(sender=self.__class__, instance=self,
+                    created=(not record_exists), raw=raw, using=using)
 
 
     save_base.alters_data = True

tests/modeltests/transactions/models.py

@@ -19 +19 @@
         ordering = ('first_name', 'last_name')
 
     def __unicode__(self):
-        return u"%s %s" % (self.first_name, self.last_name)
- No newline at end of file
+        return u"%s %s" % (self.first_name, self.last_name)
+
+class BaseModel(models.Model):
+    pass
+
+class InheritedModel(BaseModel):
+    uniq_id = models.IntegerField(unique=True)

tests/modeltests/transactions/tests.py

@@ -3 +3 @@
 from django.db import connection, transaction, IntegrityError
 from django.test import TransactionTestCase, skipUnlessDBFeature
 
-from .models import Reporter
+from .models import Reporter, InheritedModel, BaseModel
 
 
 class TransactionTests(TransactionTestCase):
@@ -160 +160 @@
             using_manually_managed_mistake
         )
 
+    @skipUnlessDBFeature('supports_transactions')
+    def test_nonmanaged_inheritance_save(self):
+        """
+        When running in nonmanaged mode, Django commits transactions after
+        each save. It multitable inheritance case, test that the commit is
+        done only once, after all tables have been saved. Check it by making
+        forcing an IntegrityError, and see what happens.
+        """
+        InheritedModel(uniq_id=1).save()
+        try:
+            InheritedModel(uniq_id=1).save()
+            # Make sure there is an error!
+            self.assertFalse(True)
+        except IntegrityError:
+            pass
+        # Now, rollback the connection and see what got saved.
+        connection.rollback()
+        self.assertEquals(InheritedModel.objects.count(), 1)
+        self.assertEquals(BaseModel.objects.count(), 1)
+
 
 class TransactionRollbackTests(TransactionTestCase):
     def execute_bad_sql(self):