From 6af05e7a0f0e4604d6a67899acaa99d73ec0dfaa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Anssi=20K=C3=A4=C3=A4ri=C3=A4inen?= <akaariai@gmail.com>
Date: Wed, 14 Aug 2013 11:05:01 +0300
Subject: [PATCH] Fixed model.__eq__ and __hash__ for no pk value cases

The __eq__ method now considers two instances without primary key value
equal only when they have same id(). The __hash__ method raises
TypeError for no primary key case.

Fixed #18864, fixed #18250

Thanks to Tim Graham for docs review.
---
 django/db/models/base.py      | 13 ++++++++++---
 django/forms/models.py        |  6 +++++-
 docs/ref/models/instances.txt | 19 +++++++++++++++++++
 docs/releases/1.7.txt         |  8 ++++++++
 tests/basic/tests.py          | 11 +++++++++++
 5 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/django/db/models/base.py b/django/db/models/base.py
index 3e2ae8d4254..a5b0f188b4d 100644
--- a/django/db/models/base.py
+++ b/django/db/models/base.py
@@ -459,14 +459,21 @@ class Model(six.with_metaclass(ModelBase)):
         return '%s object' % self.__class__.__name__
 
     def __eq__(self, other):
-        return (isinstance(other, Model) and
-                self._meta.concrete_model == other._meta.concrete_model and
-                self._get_pk_val() == other._get_pk_val())
+        if not isinstance(other, Model):
+            return False
+        if self._meta.concrete_model != other._meta.concrete_model:
+            return False
+        my_pk = self._get_pk_val()
+        if my_pk is None:
+            return self is other
+        return my_pk == other._get_pk_val()
 
     def __ne__(self, other):
         return not self.__eq__(other)
 
     def __hash__(self):
+        if self._get_pk_val() is None:
+            raise TypeError("Model instances without primary key value are unhashable")
         return hash(self._get_pk_val())
 
     def __reduce__(self):
diff --git a/django/forms/models.py b/django/forms/models.py
index a5b82e521d7..4c6ee9c6ed6 100644
--- a/django/forms/models.py
+++ b/django/forms/models.py
@@ -631,7 +631,11 @@ class BaseModelFormSet(BaseFormSet):
             seen_data = set()
             for form in valid_forms:
                 # get data for each field of each of unique_check
-                row_data = tuple([form.cleaned_data[field] for field in unique_check if field in form.cleaned_data])
+                row_data = (form.cleaned_data[field]
+                            for field in unique_check if field in form.cleaned_data)
+                # Reduce Model instances to their primary key values
+                row_data = tuple(d._get_pk_val() if hasattr(d, '_get_pk_val') else d
+                                 for d in row_data)
                 if row_data and not None in row_data:
                     # if we've already seen it then we have a uniqueness failure
                     if row_data in seen_data:
diff --git a/docs/ref/models/instances.txt b/docs/ref/models/instances.txt
index 6295eb04073..da657a9a01b 100644
--- a/docs/ref/models/instances.txt
+++ b/docs/ref/models/instances.txt
@@ -521,6 +521,25 @@ For example::
   In previous versions only instances of the exact same class and same
   primary key value were considered equal.
 
+``__hash__``
+------------
+
+.. method:: Model.__hash__()
+
+The ``__hash__`` method is based on the instance's primary key value. It
+is effectively hash(obj.pk). If the instance doesn't have a primary key
+value then a ``TypeError`` will be raised (otherwise the ``__hash__``
+method would return different values before and after the instance is
+saved, but changing the ``__hash__`` value of an instance `is forbidden
+in Python`_).
+
+.. versionchanged:: 1.7
+
+  In previous versions instance's without primary key value were
+  hashable.
+
+.. _is forbidden in Python: http://docs.python.org/reference/datamodel.html#object.__hash__
+
 ``get_absolute_url``
 --------------------
 
diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt
index 774d9d31611..6480a2505f2 100644
--- a/docs/releases/1.7.txt
+++ b/docs/releases/1.7.txt
@@ -199,6 +199,14 @@ Miscellaneous
   equal when primary keys match. Previously only instances of exact same
   class were considered equal on primary key match.
 
+* The :meth:`django.db.models.Model.__eq__` method has changed such that
+  two ``Model`` instances without primary key values won't be considered
+  equal (unless they are the same instance).
+  
+* The :meth:`django.db.models.Model.__hash__` will now raise ``TypeError``
+  when called on an instance without a primary key value. This is done to
+  avoid mutable ``__hash__`` values in containers.
+
 Features deprecated in 1.7
 ==========================
 
diff --git a/tests/basic/tests.py b/tests/basic/tests.py
index 2b051621ef4..611944902ac 100644
--- a/tests/basic/tests.py
+++ b/tests/basic/tests.py
@@ -708,9 +708,20 @@ class ModelTest(TestCase):
             SelfRef.objects.get(selfref=sr)
 
     def test_eq(self):
+        self.assertEqual(Article(id=1), Article(id=1))
         self.assertNotEqual(Article(id=1), object())
         self.assertNotEqual(object(), Article(id=1))
+        a = Article()
+        self.assertEqual(a, a)
+        self.assertNotEqual(Article(), a)
 
+    def test_hash(self):
+        # Value based on PK
+        self.assertEqual(hash(Article(id=1)), hash(1))
+        with self.assertRaises(TypeError):
+            # No PK value -> unhashable (because save() would then change
+            # hash)
+            hash(Article())
 
 class ConcurrentSaveTests(TransactionTestCase):