Fix rev_multilink properties search/retrieval

The code now only returns live (non-retired) items. Since for reverse
multilinks the Link/Multilink property in the retired item cannot be
changed, we now only return non-retired items in search (filter) and
retrieve (get).

Author: schlatterbeck

doc/customizing.txt

@@ -864,6 +864,11 @@ behaviour:
     automatically adds "1234" to the ``components`` property on
     issue3456. You can search the ``components`` multilink just like a
     regular multilink, but you can't explicitly assign to it.
+    Another difference of reverse multilinks to normal multilinks
+    is that when a linked node is retired, the node vanishes from the
+    multilink, e.g. in the example above, if an issue with ``part_of``
+    set to another issue is retired this issue vanishes from the
+    ``components`` multilink of the other issue.
 
     You can also link between different classes. So you can modify
     the issue definition to include::

roundup/backends/back_anydbm.py

@@ -1138,6 +1138,11 @@ def get(self, nodeid, propname, default=_marker, cache=1):
         # get the property (raises KeyErorr if invalid)
         prop = self.properties[propname]
 
+        if isinstance(prop, hyperdb.Multilink) and prop.computed:
+            cls = self.db.getclass(prop.rev_classname)
+            ids = cls.find(**{prop.rev_propname: nodeid})
+            return ids
+
         if propname not in d:
             if default is _marker:
                 if isinstance(prop, hyperdb.Multilink):

roundup/backends/back_mysql.py

@@ -606,11 +606,24 @@ def supports_subselects(self):
         # TODO: AFAIK its version dependent for MySQL
         return False
 
-    def _subselect(self, classname, multilink_table, nodeid_name):
+    def _subselect(self, proptree):
         ''' "I can't believe it's not a toy RDBMS"
            see, even toy RDBMSes like gadfly and sqlite can do sub-selects...
         '''
-        self.db.sql('select %s from %s'%(nodeid_name, multilink_table))
+        classname       = proptree.parent.classname
+        multilink_table = proptree.propclass.table_name
+        nodeid_name     = proptree.propclass.nodeid_name
+        linkid_name     = proptree.propclass.linkid_name
+
+        w = ''
+        if proptree.need_retired:
+            w = ' where %s.__retired__=0'%(multilink_table)
+        if proptree.need_child_retired:
+            tn1 = multilink_table
+            tn2 = '_' + proptree.classname
+            w = ', %s where %s.%s=%s.id and %s.__retired__=0'%(tn2, tn1,
+                linkid_name, tn2, tn2)
+        self.db.sql('select %s from %s%s'%(nodeid_name, multilink_table, w))
         s = ','.join([str(x[0]) for x in self.db.sql_fetchall()])
         return '_%s.id not in (%s)'%(classname, s)
 

roundup/backends/rdbms_common.py

@@ -1136,7 +1136,18 @@ def _materialize_multilink(self, classname, nodeid, node, propname):
             tn  = prop.table_name
             lid = prop.linkid_name
             nid = prop.nodeid_name
-            sql = 'select %s from %s where %s=%s' % (lid, tn, nid, self.arg)
+            w   = ''
+            joi = ''
+            if prop.computed:
+                if isinstance(prop.rev_property, Link):
+                    w = ' and %s.__retired__=0'%tn
+                else:
+                    tn2 = '_' + prop.classname
+                    joi = ', %s' % tn2
+                    w = ' and %s.%s=%s.id and %s.__retired__=0'%(tn, lid,
+                        tn2, tn2)
+            sql = 'select %s from %s%s where %s=%s%s' %(lid, tn, joi, nid,
+                self.arg, w)
             self.sql(sql, (nodeid,))
             # extract the first column from the result
             # XXX numeric ids
@@ -2343,14 +2354,26 @@ def getnodeids(self, retired=None):
         ids = [str(x[0]) for x in self.db.cursor.fetchall()]
         return ids
 
-    def _subselect(self, classname, multilink_table, nodeid_name):
+    def _subselect(self, proptree):
         """Create a subselect. This is factored out because some
            databases (hmm only one, so far) doesn't support subselects
            look for "I can't believe it's not a toy RDBMS" in the mysql
            backend.
         """
-        return '_%s.id not in (select %s from %s)'%(classname, nodeid_name,
-            multilink_table)
+        multilink_table = proptree.propclass.table_name
+        nodeid_name     = proptree.propclass.nodeid_name
+        linkid_name     = proptree.propclass.linkid_name
+        w = ''
+        if proptree.need_retired:
+            w = ' where %s.__retired__=0'%(multilink_table)
+        if proptree.need_child_retired:
+            tn1 = multilink_table
+            tn2 = '_' + proptree.classname
+            w = ', %s where %s.%s=%s.id and %s.__retired__=0'%(tn2,
+                tn1, linkid_name, tn2, tn2)
+        classname = proptree.parent.classname
+        return '_%s.id not in (select %s from %s%s)'%(classname, nodeid_name,
+            multilink_table, w)
 
     # Some DBs order NULL values last. Set this variable in the backend
     # for prepending an order by clause for each attribute that causes
@@ -2516,7 +2539,7 @@ def _filter_sql (self, search_matches, filterspec, srt=[], grp=[], retr=0,
                     if v in ('-1', ['-1'], []):
                         # only match rows that have count(linkid)=0 in the
                         # corresponding multilink table)
-                        where.append(self._subselect(pcn, tn, nid))
+                        where.append(self._subselect(p))
                     else:
                         frum.append(tn)
                         gen_join = True
@@ -2530,9 +2553,11 @@ def _filter_sql (self, search_matches, filterspec, srt=[], grp=[], retr=0,
                         if gen_join:
                             where.append('_%s.id=%s.%s'%(pln, tn, nid))
 
-                        if p.children:
+                        if p.children or p.need_child_retired:
                             frum.append('_%s as _%s' % (cn, ln))
                             where.append('%s.%s=_%s.id'%(tn, lid, ln))
+                            if p.need_child_retired:
+                                where.append('_%s.__retired__=0'%(ln))
 
                         if p.has_values:
                             if isinstance(v, type([])):
@@ -2541,6 +2566,9 @@ def _filter_sql (self, search_matches, filterspec, srt=[], grp=[], retr=0,
                             else:
                                 where.append('%s.%s=%s'%(tn, lid, a))
                                 args.append(v)
+                        # Don't match retired nodes if rev_multilink
+                        if p.need_retired:
+                            where.append('%s.__retired__=0'%(tn))
                 if 'sort' in p.need_for:
                     assert not p.attr_sort_done and not p.sort_ids_needed
             elif k == 'id':

roundup/hyperdb.py

@@ -489,6 +489,8 @@ def __init__(self, db, cls, name, props, parent=None, retr=False):
         self.propclass = None
         self.orderby = []
         self.sql_idx = None  # index of retrieved column in sql result
+        self.need_retired = False
+        self.need_child_retired = False
         if parent:
             self.root = parent.root
             self.depth = parent.depth + 1
@@ -526,6 +528,11 @@ def append(self, name, need_for='search', retr=False):
         child = self.__class__(self.db, cls, name, props, parent=self)
         child.need_for = {need_for: True}
         child.propclass = propclass
+        if isinstance(propclass, Multilink) and self.props[name].computed:
+            if isinstance(self.props[name].rev_property, Link):
+                child.need_retired = True
+            else:
+                child.need_child_retired = True
         self.children.append(child)
         self.propdict[name] = child
         if retr and isinstance(child.propclass, Link):
@@ -594,9 +601,11 @@ def search(self, search_matches=None, sort=True, retired=False):
                                     s2.add(node [pn])
                         items = s1.difference(s2)
                     elif isinstance(p.propclass.rev_property, Link):
-                        items = set(cl.get(x, pn) for x in p.val)
+                        items = set(cl.get(x, pn) for x in p.val
+                            if not cl.is_retired(x))
                     else:
-                        items = set().union(*(cl.get(x, pn) for x in p.val))
+                        items = set().union(*(cl.get(x, pn) for x in p.val
+                            if not cl.is_retired(x)))
                     filterspec[p.name] = list(sorted(items))
                 elif isinstance(p.val, type([])):
                     exact = []

test/db_test_base.py

@@ -1862,6 +1862,27 @@ def testFilteringRevLink(self):
             ae(filt(None, {'issues.title': ['ts2']}), ['6'])
             ae(filt(None, {'issues': ['-1']}), ['1', '2', '3', '4', '5'])
             ae(filt(None, {'issues': '-1'}), ['1', '2', '3', '4', '5'])
+        def ls(x):
+            return list(sorted(x))
+        self.assertEqual(ls(self.db.user.get('6', 'issues')), ['1', '2'])
+        self.assertEqual(ls(self.db.user.get('7', 'issues')), ['3'])
+        self.assertEqual(ls(self.db.user.get('10', 'issues')), ['6', '7', '8'])
+        n = self.db.user.getnode('6')
+        self.assertEqual(ls(n.issues), ['1', '2'])
+        # Now retire some linked-to issues and retry
+        self.db.issue.retire('6')
+        self.db.issue.retire('2')
+        self.db.issue.retire('3')
+        self.db.commit()
+        for filt in filter, filter_iter:
+            ae(filt(None, {'issues': ['3', '4']}), ['8'])
+            ae(filt(None, {'issues': ['1', '4', '8']}), ['6', '8', '10'])
+            ae(filt(None, {'issues.title': ['ts2']}), [])
+            ae(filt(None, {'issues': ['-1']}), ['1', '2', '3', '4', '5', '7'])
+            ae(filt(None, {'issues': '-1'}), ['1', '2', '3', '4', '5', '7'])
+        self.assertEqual(ls(self.db.user.get('6', 'issues')), ['1'])
+        self.assertEqual(ls(self.db.user.get('7', 'issues')), [])
+        self.assertEqual(ls(self.db.user.get('10', 'issues')), ['7', '8'])
 
     def testFilteringLinkSortSearchMultilink(self):
         ae, filter, filter_iter = self.filteringSetup()
@@ -1912,8 +1933,29 @@ def testFilteringRevMultilink(self):
         for filt in filter, filter_iter:
             ae(filt(None, {ni: ['1', '2']}), ['4', '5'])
             ae(filt(None, {ni: ['6','7']}), ['3', '4', '5'])
+            ae(filt(None, {'nosy_issues.title': ['ts2']}), ['5'])
             ae(filt(None, {ni: ['-1']}), ['1', '2', '6', '7', '8', '9', '10'])
             ae(filt(None, {ni: '-1'}), ['1', '2', '6', '7', '8', '9', '10'])
+        def ls(x):
+            return list(sorted(x))
+        self.assertEqual(ls(self.db.user.get('4', ni)), ['1', '6'])
+        self.assertEqual(ls(self.db.user.get('5', ni)), ['2', '6', '7'])
+        n = self.db.user.getnode('4')
+        self.assertEqual(ls(n.nosy_issues), ['1', '6'])
+        # Now retire some linked-to issues and retry
+        self.db.issue.retire('2')
+        self.db.issue.retire('6')
+        self.db.commit()
+        for filt in filter, filter_iter:
+            ae(filt(None, {ni: ['1', '2']}), ['4'])
+            ae(filt(None, {ni: ['6','7']}), ['5'])
+            ae(filt(None, {'nosy_issues.title': ['ts2']}), [])
+            ae(filt(None, {ni: ['-1']}),
+                ['1', '2', '3', '6', '7', '8', '9', '10'])
+            ae(filt(None, {ni: '-1'}),
+                ['1', '2', '3', '6', '7', '8', '9', '10'])
+        self.assertEqual(ls(self.db.user.get('4', ni)), ['1'])
+        self.assertEqual(ls(self.db.user.get('5', ni)), ['7'])
 
     def testFilteringMany(self):
         ae, filter, filter_iter = self.filteringSetup()