launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:stormbase-eq-hash into launchpad:master.

Commit message:
Implement __eq__, __ne__, and __hash__ for StormBase

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

This matches SQLBase: it's much less confusing if converting a model to Storm doesn't quietly change its equality and hashing semantics.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:stormbase-eq-hash into launchpad:master.

diff --git a/lib/lp/services/database/stormbase.py b/lib/lp/services/database/stormbase.py
index 97b85e6..307333d 100644
--- a/lib/lp/services/database/stormbase.py
+++ b/lib/lp/services/database/stormbase.py
@@ -6,7 +6,12 @@ __all__ = [
     'StormBase',
     ]
 
-from storm.base import Storm
+from storm.info import get_obj_info
+from storm.locals import (
+    Store,
+    Storm,
+    )
+from zope.security.proxy import removeSecurityProxy
 
 from lp.services.propertycache import clear_property_cache
 
@@ -17,6 +22,48 @@ class StormBase(Storm):
     This class adds storm cache management functions to base.Storm.
     """
 
+    def __eq__(self, other):
+        """Equality operator.
+
+        Objects compare equal if they have the same class and id, and the id
+        is not None.
+
+        This rule allows objects retrieved from different stores to compare
+        equal.  Newly-created objects may not yet have an id; in such cases
+        we flush the store so that we can find out their id.
+        """
+        other = removeSecurityProxy(other)
+        if self.__class__ != other.__class__:
+            return False
+        self_obj_info = get_obj_info(self)
+        other_obj_info = get_obj_info(other)
+        if "primary_vars" not in self_obj_info:
+            Store.of(self).flush()
+        if "primary_vars" not in other_obj_info:
+            Store.of(other).flush()
+        self_primary = [var.get() for var in self_obj_info["primary_vars"]]
+        other_primary = [var.get() for var in other_obj_info["primary_vars"]]
+        return self_primary == other_primary
+
+    def __ne__(self, other):
+        """Inverse of __eq__."""
+        return not (self == other)
+
+    def __hash__(self):
+        """Hash operator.
+
+        We must define __hash__ since we define __eq__ (Python 3 requires
+        this), but we need to take care to preserve the invariant that
+        objects that compare equal have the same hash value.  Newly-created
+        objects may not yet have an id; in such cases we flush the store so
+        that we can find out their id.
+        """
+        obj_info = get_obj_info(self)
+        if "primary_vars" not in obj_info:
+            Store.of(self).flush()
+        primary = [var.get() for var in obj_info["primary_vars"]]
+        return hash((self.__class__,) + tuple(primary))
+
     # XXX: jcsackett 2011-01-20 bug=622648: Much as with the SQLBase,
     # this is not tested.
     def __storm_invalidated__(self):
diff --git a/lib/lp/services/database/tests/test_collection.py b/lib/lp/services/database/tests/test_collection.py
index 391f9b7..be7eaeb 100644
--- a/lib/lp/services/database/tests/test_collection.py
+++ b/lib/lp/services/database/tests/test_collection.py
@@ -43,6 +43,7 @@ def make_table(range_start, range_end, table_name=None):
 
         def __init__(self, id):
             self.id = id
+            IStore(self.__class__).add(self)
 
         def __eq__(self, other):
             return self.id == other.id