Ticket #17332: 17332.diff

django/db/models/base.py

@@ -2 +2 @@
 import sys
 from functools import update_wrapper
 from itertools import izip
+import warnings
 
 import django.db.models.manager     # Imported to register signal handler.
 from django.conf import settings
@@ -271 +272 @@
         # 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 = True
+        self.old_pk_values = {}
+
+    def update(self, obj, using, adding=False, pk_fields=None):
+        if pk_fields is None:
+            pk_fields = obj._meta.pk_fields
+        self.db = using
+        self.adding = adding
+        for f in pk_fields:
+            self.old_pk_values[f.attname] = f.to_immutable_value(obj)
+
+    def clear(self):
+        self.db = None
+        self.adding = True
+        self.old_pk_values = {}
 
 class Model(object):
     __metaclass__ = ModelBase
@@ -429 +444 @@
         return getattr(self, meta.pk.attname)
 
     def _set_pk_val(self, value):
+        if value is None:
+            self._state.old_pk_values[self._meta.pk.attname] = None
         return setattr(self, self._meta.pk.attname, value)
 
     pk = property(_get_pk_val, _set_pk_val)
@@ -486 +503 @@
         if origin and not meta.auto_created:
             signals.pre_save.send(sender=origin, instance=self, raw=raw, using=using)
 
+        # Proxy-objects do not need saving. Skip to the parent objects.
+        if meta.proxy:
+            for parent, _ in meta.parents.items():
+                self.save_base(cls=parent, origin=cls, using=using)
+            return
         # 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
@@ -504 +520 @@
                 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, origin=None, 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]
+        pk_val = self._get_pk_val(meta)
+        pk_set = pk_val is not None
+        record_exists = True
+        manager = cls._base_manager
+
+        if pk_set and not self._state.adding:
+            old_pk_val = self._state.old_pk_values[meta.pk.attname]
+            if old_pk_val is not None and pk_val != old_pk_val:
+                warnings.warn("Saving an object with changed primary key is deprecated. "
+                              "You can use obj.pk = None; obj.pk = new_val to bypass this check.",
+                              PendingDeprecationWarning)
+
+        # First, try an UPDATE. If that doesn't update anything, do an INSERT.
+        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
-
-                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)
+        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)]
+
+            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)
+
+        if origin:
+            # We are at the topmost object - finalize the save
             transaction.commit_unless_managed(using=using)
-
-        # 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)
-
+            self._state.update(self, using)
+            if not meta.auto_created:
+                # Signal that the save is complete
+                signals.post_save.send(sender=origin, instance=self,
+                    created=(not record_exists), raw=raw, using=using)
 
     save_base.alters_data = True
 
@@ -575 +592 @@
         collector = Collector(using=using)
         collector.collect([self])
         collector.delete()
+        self._state.clear()
 
     delete.alters_data = True
 

django/db/models/fields/__init__.py

@@ -428 +428 @@
         """
         return smart_unicode(self._get_val_from_obj(obj))
 
+    def to_immutable_value(self, obj):
+        """
+        Returns a value which is guaranteed to be immutable. This is used to
+        track changes in field values. The returned value should be usable in
+        database lookups.
+        """
+        return getattr(obj, self.attname)
+
     def bind(self, fieldmapping, original, bound_field_class):
         return bound_field_class(self, fieldmapping, original)
 

django/db/models/fields/files.py

@@ -249 +249 @@
             file.save(file.name, file, save=False)
         return file
 
+    def to_immutable_value(self, obj):
+        return getattr(obj, self.attname).name
+
     def contribute_to_class(self, cls, name):
         super(FileField, self).contribute_to_class(cls, name)
         setattr(cls, self.name, self.descriptor_class(self))

django/db/models/options.py

@@ -35 +35 @@
         self.db_tablespace = settings.DEFAULT_TABLESPACE
         self.admin = None
         self.meta = meta
+        # The PK of the model's table
         self.pk = None
         self.has_auto_field, self.auto_field = False, None
         self.abstract = False
@@ -105 +106 @@
             self.db_table = "%s_%s" % (self.app_label, self.module_name)
             self.db_table = truncate_name(self.db_table, connection.ops.max_name_length())
 
+
     def _prepare(self, model):
         if self.order_with_respect_to:
             self.order_with_respect_to = self.get_field(self.order_with_respect_to)
@@ -164 +166 @@
             if hasattr(self, '_field_cache'):
                 del self._field_cache
                 del self._field_name_cache
+                del self._pk_field_cache
 
         if hasattr(self, '_name_map'):
             del self._name_map
@@ -219 +222 @@
         return self._field_name_cache
     fields = property(_fields)
 
+    def _pk_fields(self):
+        """
+        All the PKs the model has, including parent tables.
+        """
+        try:
+            self._field_name_cache
+        except AttributeError:
+            self._fill_fields_cache()
+        return self._pk_field_cache
+    pk_fields = property(_pk_fields)
+
     def get_fields_with_model(self):
         """
         Returns a sequence of (field, model) pairs for all fields. The "model"
@@ -242 +256 @@
         cache.extend([(f, None) for f in self.local_fields])
         self._field_cache = tuple(cache)
         self._field_name_cache = [x for x, _ in cache]
+        self._pk_field_cache = tuple(x for x, _ in cache if x.primary_key)
 
     def _many_to_many(self):
         try:

django/db/models/query.py

@@ -278 +278 @@
                     init_list.append(field.attname)
             model_cls = deferred_class_factory(self.model, skip)
 
-        # Cache db and model outside the loop
+        # Cache some values outside the loop
         db = self.db
         model = self.model
         compiler = self.query.get_compiler(using=db)
+        pk_fields = model._meta.pk_fields
         if fill_cache:
             klass_info = get_klass_info(model, max_depth=max_depth,
                                         requested=requested, only_load=only_load)
@@ -297 +298 @@
                     # Omit aggregates in object creation.
                     obj = model(*row[index_start:aggregate_start])
 
-                # Store the source database of the object
-                obj._state.db = db
-                # This object came from the database; it's not being added.
-                obj._state.adding = False
+                obj._state.update(obj, db, pk_fields=pk_fields)
 
             if extra_select:
                 for i, k in enumerate(extra_select):
@@ -1316 +1314 @@
     return klass, field_names, field_count, related_fields, reverse_related_fields
 
 
-def get_cached_row(row, index_start, using,  klass_info, offset=0):
+def get_cached_row(row, index_start, using, klass_info, offset=0):
     """
     Helper function that recursively returns an object with the specified
     related attributes already populated.
@@ -1352 +1350 @@
 
     # If an object was retrieved, set the database state.
     if obj:
-        obj._state.db = using
-        obj._state.adding = False
+        obj._state.update(obj, using)
 
     # Instantiate related fields
     index_end = index_start + field_count + offset
@@ -1486 +1483 @@
                 for column, pos in annotation_fields:
                     setattr(instance, column, values[pos])
 
-            instance._state.db = db
-            instance._state.adding = False
-
+            instance._state.update(instance, db)
             yield instance
 
     def __repr__(self):