Mercurial > p > roundup > code
changeset 4490:559d9a2a0191
Fixed bug in filter_iter refactoring (lazy multilinks)...
...in rare cases this would result in duplicate multilinks to the same
node. We're now going the safe route and doing lazy evaluation only
for read-only access, whenever updates are done we fetch everything.
| author | Ralf Schlatterbeck <schlatterbeck@users.sourceforge.net> |
|---|---|
| date | Sat, 16 Apr 2011 11:02:01 +0000 |
| parents | 47bd330e3d17 |
| children | 357c6079c73b |
| files | CHANGES.txt roundup/backends/rdbms_common.py test/db_test_base.py |
| diffstat | 3 files changed, 55 insertions(+), 10 deletions(-) [+] |
line wrap: on
line diff
--- a/CHANGES.txt Fri Apr 15 19:04:08 2011 +0000 +++ b/CHANGES.txt Sat Apr 16 11:02:01 2011 +0000 @@ -96,6 +96,10 @@ parameter for binding to all interfaces now (still left in for compatibility). Thanks to Toni Mueller for providing the first version of this patch and discussing implementations. +- Fixed bug in filter_iter refactoring (lazy multilinks), in rare cases + this would result in duplicate multilinks to the same node. We're now + going the safe route and doing lazy evaluation only for read-only + access, whenever updates are done we fetch everything. 2010-10-08 1.4.16 (r4541)
--- a/roundup/backends/rdbms_common.py Fri Apr 15 19:04:08 2011 +0000 +++ b/roundup/backends/rdbms_common.py Sat Apr 16 11:02:01 2011 +0000 @@ -1080,8 +1080,34 @@ raise ValueError('%r is not a hyperdb property class' % propklass) - def getnode(self, classname, nodeid): + def _materialize_multilink(self, classname, nodeid, node, propname): + """ evaluation of single Multilink (lazy eval may have skipped this) + """ + if propname not in node: + sql = 'select linkid from %s_%s where nodeid=%s'%(classname, + propname, self.arg) + self.sql(sql, (nodeid,)) + # extract the first column from the result + # XXX numeric ids + items = [int(x[0]) for x in self.cursor.fetchall()] + items.sort () + node[propname] = [str(x) for x in items] + + def _materialize_multilinks(self, classname, nodeid, node, props=None): + """ get all Multilinks of a node (lazy eval may have skipped this) + """ + cl = self.classes[classname] + props = props or [pn for (pn, p) in cl.properties.iteritems() + if isinstance(p, Multilink)] + for propname in props: + if propname not in node: + self._materialize_multilink(classname, nodeid, node, propname) + + def getnode(self, classname, nodeid, fetch_multilinks=True): """ Get a node from the database. + For optimisation optionally we don't fetch multilinks + (lazy Multilinks). + But for internal database operations we need them. """ # see if we have this node cached key = (classname, nodeid) @@ -1091,6 +1117,8 @@ if __debug__: self.stats['cache_hits'] += 1 # return the cached information + if fetch_multilinks: + self._materialize_multilinks(classname, nodeid, self.cache[key]) return self.cache[key] if __debug__: @@ -1124,6 +1152,9 @@ value = self.to_hyperdb_value(props[name].__class__)(value) node[name] = value + if fetch_multilinks and mls: + self._materialize_multilinks(classname, nodeid, node, mls) + # save off in the cache key = (classname, nodeid) self._cache_save(key, node) @@ -1616,7 +1647,7 @@ return nodeid # get the node's dict - d = self.db.getnode(self.classname, nodeid) + d = self.db.getnode(self.classname, nodeid, fetch_multilinks=False) # handle common case -- that property is in dict -- first # if None and one of creator/creation actor/activity return None if propname in d: @@ -1640,14 +1671,7 @@ # lazy evaluation of Multilink if propname not in d and isinstance(prop, Multilink): - sql = 'select linkid from %s_%s where nodeid=%s'%(self.classname, - propname, self.db.arg) - self.db.sql(sql, (nodeid,)) - # extract the first column from the result - # XXX numeric ids - items = [int(x[0]) for x in self.db.cursor.fetchall()] - items.sort () - d[propname] = [str(x) for x in items] + self.db._materialize_multilink(self.classname, nodeid, d, propname) # handle there being no value in the table for the property if propname not in d or d[propname] is None:
--- a/test/db_test_base.py Fri Apr 15 19:04:08 2011 +0000 +++ b/test/db_test_base.py Sat Apr 16 11:02:01 2011 +0000 @@ -321,6 +321,23 @@ if commit: self.db.commit() self.assertEqual(self.db.issue.get(nid, "nosy"), []) + def testMakeSeveralMultilinkedNodes(self): + for commit in (0,1): + u1 = self.db.user.create(username='foo%s'%commit) + u2 = self.db.user.create(username='bar%s'%commit) + u3 = self.db.user.create(username='baz%s'%commit) + nid = self.db.issue.create(title="spam", nosy=[u1]) + if commit: self.db.commit() + self.assertEqual(self.db.issue.get(nid, "nosy"), [u1]) + self.db.issue.set(nid, deadline=date.Date('.')) + self.db.issue.set(nid, nosy=[u1,u2], title='ta%s'%commit) + if commit: self.db.commit() + self.assertEqual(self.db.issue.get(nid, "nosy"), [u1,u2]) + self.db.issue.set(nid, deadline=date.Date('.')) + self.db.issue.set(nid, nosy=[u1,u2,u3], title='tb%s'%commit) + if commit: self.db.commit() + self.assertEqual(self.db.issue.get(nid, "nosy"), [u1,u2,u3]) + def testMultilinkChangeIterable(self): for commit in (0,1): # invalid nosy value assertion
