Merge ~cjwatson/launchpad:stormbase-eq-hash into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 6f2a36c1e74d99f511b033c19273c4cb2b67c6c4
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:stormbase-eq-hash
Merge into: launchpad:master
Diff against target: 91 lines (+51/-3)
2 files modified
lib/lp/services/database/stormbase.py (+49/-2)
lib/lp/services/database/tests/test_collection.py (+2/-1)
Reviewer Review Type Date Requested Status
Cristian Gonzalez (community) Approve
Review via email: mp+398087@code.launchpad.net

Commit message

Implement __eq__, __ne__, and __hash__ for StormBase

Description of the change

This matches SQLBase: it's much less confusing if converting a model to Storm doesn't quietly change its equality and hashing semantics.

To post a comment you must log in.
Revision history for this message
Cristian Gonzalez (cristiangsp) wrote :

Looks good!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/services/database/stormbase.py b/lib/lp/services/database/stormbase.py
2index 97b85e6..4d99064 100644
3--- a/lib/lp/services/database/stormbase.py
4+++ b/lib/lp/services/database/stormbase.py
5@@ -1,4 +1,4 @@
6-# Copyright 2011 Canonical Ltd. This software is licensed under the
7+# Copyright 2011-2021 Canonical Ltd. This software is licensed under the
8 # GNU Affero General Public License version 3 (see the file LICENSE).
9
10 __metaclass__ = type
11@@ -6,7 +6,12 @@ __all__ = [
12 'StormBase',
13 ]
14
15-from storm.base import Storm
16+from storm.info import get_obj_info
17+from storm.locals import (
18+ Store,
19+ Storm,
20+ )
21+from zope.security.proxy import removeSecurityProxy
22
23 from lp.services.propertycache import clear_property_cache
24
25@@ -17,6 +22,48 @@ class StormBase(Storm):
26 This class adds storm cache management functions to base.Storm.
27 """
28
29+ def __eq__(self, other):
30+ """Equality operator.
31+
32+ Objects compare equal if they have the same class and id, and the id
33+ is not None.
34+
35+ This rule allows objects retrieved from different stores to compare
36+ equal. Newly-created objects may not yet have an id; in such cases
37+ we flush the store so that we can find out their id.
38+ """
39+ other = removeSecurityProxy(other)
40+ if self.__class__ != other.__class__:
41+ return False
42+ self_obj_info = get_obj_info(self)
43+ other_obj_info = get_obj_info(other)
44+ if "primary_vars" not in self_obj_info:
45+ Store.of(self).flush()
46+ if "primary_vars" not in other_obj_info:
47+ Store.of(other).flush()
48+ self_primary = [var.get() for var in self_obj_info["primary_vars"]]
49+ other_primary = [var.get() for var in other_obj_info["primary_vars"]]
50+ return self_primary == other_primary
51+
52+ def __ne__(self, other):
53+ """Inverse of __eq__."""
54+ return not (self == other)
55+
56+ def __hash__(self):
57+ """Hash operator.
58+
59+ We must define __hash__ since we define __eq__ (Python 3 requires
60+ this), but we need to take care to preserve the invariant that
61+ objects that compare equal have the same hash value. Newly-created
62+ objects may not yet have an id; in such cases we flush the store so
63+ that we can find out their id.
64+ """
65+ obj_info = get_obj_info(self)
66+ if "primary_vars" not in obj_info:
67+ Store.of(self).flush()
68+ primary = [var.get() for var in obj_info["primary_vars"]]
69+ return hash((self.__class__,) + tuple(primary))
70+
71 # XXX: jcsackett 2011-01-20 bug=622648: Much as with the SQLBase,
72 # this is not tested.
73 def __storm_invalidated__(self):
74diff --git a/lib/lp/services/database/tests/test_collection.py b/lib/lp/services/database/tests/test_collection.py
75index 391f9b7..f388556 100644
76--- a/lib/lp/services/database/tests/test_collection.py
77+++ b/lib/lp/services/database/tests/test_collection.py
78@@ -1,4 +1,4 @@
79-# Copyright 2010 Canonical Ltd. This software is licensed under the
80+# Copyright 2010-2021 Canonical Ltd. This software is licensed under the
81 # GNU Affero General Public License version 3 (see the file LICENSE).
82
83 """Tests for `Collection`."""
84@@ -43,6 +43,7 @@ def make_table(range_start, range_end, table_name=None):
85
86 def __init__(self, id):
87 self.id = id
88+ IStore(self.__class__).add(self)
89
90 def __eq__(self, other):
91 return self.id == other.id

Subscribers

People subscribed via source and target branches

to status/vote changes: