launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:py3-more-hashable into launchpad:master.

Commit message:
Make more classes hashable on Python 3

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/398244

Python 3 considers objects unhashable if their class overrides __eq__ but not __hash__, so add suitable __hash__ methods where necessary.  I also took the opportunity to remove some __eq__ methods that are now superseded by StormBase.__eq__.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:py3-more-hashable into launchpad:master.

diff --git a/lib/lp/oci/model/ocirecipe.py b/lib/lp/oci/model/ocirecipe.py
index 41493ee..fd76ae6 100644
--- a/lib/lp/oci/model/ocirecipe.py
+++ b/lib/lp/oci/model/ocirecipe.py
@@ -735,3 +735,6 @@ class OCIRecipeBuildRequest:
         if not zope_isinstance(other, self.__class__):
             return False
         return self.id == other.id
+
+    def __hash__(self):
+        return hash((self.__class__, self.id))
diff --git a/lib/lp/oci/model/ocirecipebuild.py b/lib/lp/oci/model/ocirecipebuild.py
index 9163245..9ebb2e0 100644
--- a/lib/lp/oci/model/ocirecipebuild.py
+++ b/lib/lp/oci/model/ocirecipebuild.py
@@ -24,16 +24,12 @@ from storm.locals import (
     Or,
     Reference,
     Store,
-    Storm,
     Unicode,
     )
 from storm.store import EmptyResultSet
 from zope.component import getUtility
 from zope.interface import implementer
-from zope.security.proxy import (
-    isinstance as zope_isinstance,
-    removeSecurityProxy,
-    )
+from zope.security.proxy import removeSecurityProxy
 
 from lp.app.errors import NotFoundError
 from lp.buildmaster.enums import (
@@ -71,6 +67,7 @@ from lp.services.database.interfaces import (
     IMasterStore,
     IStore,
     )
+from lp.services.database.stormbase import StormBase
 from lp.services.job.interfaces.job import JobStatus
 from lp.services.job.model.job import Job
 from lp.services.librarian.model import (
@@ -91,7 +88,7 @@ from lp.services.webapp.snapshot import notify_modified
 
 
 @implementer(IOCIFile)
-class OCIFile(Storm):
+class OCIFile(StormBase):
 
     __storm_table__ = 'OCIFile'
 
@@ -127,7 +124,7 @@ class OCIFileSet:
 
 
 @implementer(IOCIRecipeBuild)
-class OCIRecipeBuild(PackageBuildMixin, Storm):
+class OCIRecipeBuild(PackageBuildMixin, StormBase):
 
     __storm_table__ = 'OCIRecipeBuild'
 
@@ -190,11 +187,6 @@ class OCIRecipeBuild(PackageBuildMixin, Storm):
         if build_request is not None:
             self.build_request_id = build_request.id
 
-    def __eq__(self, other):
-        if not zope_isinstance(other, self.__class__):
-            return False
-        return self.id == other.id
-
     @property
     def build_request(self):
         """See `IOCIRecipeBuild`."""
diff --git a/lib/lp/registry/model/persondistributionsourcepackage.py b/lib/lp/registry/model/persondistributionsourcepackage.py
index 8ae9ebb..7dfa6c2 100644
--- a/lib/lp/registry/model/persondistributionsourcepackage.py
+++ b/lib/lp/registry/model/persondistributionsourcepackage.py
@@ -41,8 +41,11 @@ class PersonDistributionSourcePackage:
     def __eq__(self, other):
         return (
             IPersonDistributionSourcePackage.providedBy(other) and
-            self.person.id == other.person.id and
+            self.person == other.person and
             self.distro_source_package == other.distro_source_package)
 
     def __ne__(self, other):
         return not self == other
+
+    def __hash__(self):
+        return hash((self.person, self.distro_source_package))
diff --git a/lib/lp/registry/model/personociproject.py b/lib/lp/registry/model/personociproject.py
index a1137fe..3a1ec86 100644
--- a/lib/lp/registry/model/personociproject.py
+++ b/lib/lp/registry/model/personociproject.py
@@ -39,8 +39,11 @@ class PersonOCIProject:
     def __eq__(self, other):
         return (
             IPersonOCIProject.providedBy(other) and
-            self.person.id == other.person.id and
+            self.person == other.person and
             self.oci_project == other.oci_project)
 
     def __ne__(self, other):
         return not self == other
+
+    def __hash__(self):
+        return hash((self.person, self.oci_project))
diff --git a/lib/lp/registry/model/personproduct.py b/lib/lp/registry/model/personproduct.py
index 16d7ff6..6cade57 100644
--- a/lib/lp/registry/model/personproduct.py
+++ b/lib/lp/registry/model/personproduct.py
@@ -37,12 +37,15 @@ class PersonProduct(HasMergeProposalsMixin):
     def __eq__(self, other):
         return (
             IPersonProduct.providedBy(other) and
-            self.person.id == other.person.id and
-            self.product.id == other.product.id)
+            self.person == other.person and
+            self.product == other.product)
 
     def __ne__(self, other):
         return not self == other
 
+    def __hash__(self):
+        return hash((self.person, self.product))
+
     @property
     def private(self):
         return self.person.private or self.product.private
diff --git a/lib/lp/registry/model/suitesourcepackage.py b/lib/lp/registry/model/suitesourcepackage.py
index 914da7e..02a99c9 100644
--- a/lib/lp/registry/model/suitesourcepackage.py
+++ b/lib/lp/registry/model/suitesourcepackage.py
@@ -34,6 +34,9 @@ class SuiteSourcePackage:
     def __ne__(self, other):
         return not (self == other)
 
+    def __hash__(self):
+        return hash((self.sourcepackage, self.pocket))
+
     def __repr__(self):
         return '<%s %s>' % (self.__class__.__name__, self.path)
 
diff --git a/lib/lp/services/database/tests/test_collection.py b/lib/lp/services/database/tests/test_collection.py
index f388556..6049ed0 100644
--- a/lib/lp/services/database/tests/test_collection.py
+++ b/lib/lp/services/database/tests/test_collection.py
@@ -5,14 +5,12 @@
 
 __metaclass__ = type
 
-from storm.locals import (
-    Int,
-    Storm,
-    )
+from storm.locals import Int
 
 from lp.registry.model.person import Person
 from lp.services.database.collection import Collection
 from lp.services.database.interfaces import IStore
+from lp.services.database.stormbase import StormBase
 from lp.testing import TestCaseWithFactory
 from lp.testing.fakemethod import FakeMethod
 from lp.testing.layers import ZopelessDatabaseLayer
@@ -36,7 +34,7 @@ def make_table(range_start, range_end, table_name=None):
        FROM generate_series(%d, %d)
        """ % (table_name, range_start, range_end - 1))
 
-    class TestTable(Storm):
+    class TestTable(StormBase):
         """A test class/table generated on the fly for testing purposes."""
         __storm_table__ = table_name
         id = Int(primary=True)
@@ -45,9 +43,6 @@ def make_table(range_start, range_end, table_name=None):
             self.id = id
             IStore(self.__class__).add(self)
 
-        def __eq__(self, other):
-            return self.id == other.id
-
     return TestTable
 
 
diff --git a/lib/lp/services/job/runner.py b/lib/lp/services/job/runner.py
index 15ea4e9..4d302d5 100644
--- a/lib/lp/services/job/runner.py
+++ b/lib/lp/services/job/runner.py
@@ -155,6 +155,9 @@ class BaseRunnableJob(BaseRunnableJobSource):
     def __ne__(self, job):
         return not (self == job)
 
+    def __hash__(self):
+        return hash(tuple([self.__class__] + sorted(self.__dict__.items())))
+
     def __lt__(self, job):
         naked_job = removeSecurityProxy(job)
         if self.__class__ is naked_job.__class__: