Commit 6af05e7a authored by Anssi Kääriäinen's avatar Anssi Kääriäinen
Browse files

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.
parent 768bbf3e

django/db/models/base.py

+10 −3
Original line number Diff line number Diff line
@@ -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):

django/forms/models.py

+5 −1
Original line number Diff line number Diff line
@@ -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:

docs/ref/models/instances.txt

+19 −0
Original line number Diff line number Diff line
@@ -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``

--------------------

docs/releases/1.7.txt

+8 −0
Original line number Diff line number Diff line
@@ -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

==========================

tests/basic/tests.py

+11 −0
Original line number Diff line number Diff line
@@ -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):