[rset] fix infinite recursion introduced in c1eb5a676c80
The mechanism to avoid loops in c1eb5a676c80 "[rset] Always complete
attribute/relation caches in ResultSet.get_entity" breaks down: we would
loop forever if two entities that were already in the cache were linked
by a relation with ? or 1 cardinality in both directions. To avoid
that, keep a set of already-considered columns. Related to #9942503.
--- a/cubicweb/rset.py Wed Feb 17 10:02:03 2016 +0100
+++ b/cubicweb/rset.py Mon Feb 29 12:32:52 2016 +0100
@@ -470,7 +470,7 @@
self.req.set_entity_cache(entity)
return entity
- def _build_entity(self, row, col):
+ def _build_entity(self, row, col, seen=None):
"""internal method to get a single entity, returns a partially
initialized Entity instance.
@@ -484,16 +484,13 @@
:return: the partially initialized `Entity` instance
"""
req = self.req
- if req is None:
- raise AssertionError('dont call get_entity with no req on the result set')
+ assert req is not None, 'do not call get_entity with no req on the result set'
+
rowvalues = self.rows[row]
eid = rowvalues[col]
assert eid is not None
try:
entity = req.entity_cache(eid)
- if entity.cw_rset is self:
- # return entity as is, avoiding recursion
- return entity
except KeyError:
entity = self._make_entity(row, col)
else:
@@ -504,6 +501,12 @@
entity.cw_rset = self
entity.cw_row = row
entity.cw_col = col
+ # avoid recursion
+ if seen is None:
+ seen = set()
+ if col in seen:
+ return entity
+ seen.add(col)
# try to complete the entity if there are some additional columns
if len(rowvalues) > 1:
eschema = entity.e_schema
@@ -521,7 +524,7 @@
rrset = ResultSet([], rql % (rtype, entity.eid))
rrset.req = req
else:
- rrset = self._build_entity(row, col_idx).as_rset()
+ rrset = self._build_entity(row, col_idx, seen).as_rset()
entity.cw_set_relation_cache(rtype, role, rrset)
return entity
--- a/cubicweb/test/unittest_rset.py Wed Feb 17 10:02:03 2016 +0100
+++ b/cubicweb/test/unittest_rset.py Mon Feb 29 12:32:52 2016 +0100
@@ -360,6 +360,29 @@
self.assertEqual(s.cw_attr_cache['name'], 'activated')
self.assertRaises(KeyError, s.cw_attr_cache.__getitem__, 'description')
+ def test_get_entity_recursion(self):
+ with self.admin_access.repo_cnx() as cnx:
+ cnx.create_entity('EmailAddress', address=u'toto',
+ reverse_primary_email=cnx.user.eid)
+ cnx.commit()
+
+ # get_entity should fill the caches for user and email, even if both
+ # entities are already in the connection's entity cache
+ with self.admin_access.repo_cnx() as cnx:
+ mail = cnx.find('EmailAddress').one()
+ rset = cnx.execute('Any X, E WHERE X primary_email E')
+ u = rset.get_entity(0, 0)
+ self.assertTrue(u.cw_relation_cached('primary_email', 'subject'))
+ self.assertTrue(mail.cw_relation_cached('primary_email', 'object'))
+
+ with self.admin_access.repo_cnx() as cnx:
+ mail = cnx.find('EmailAddress').one()
+ rset = cnx.execute('Any X, E WHERE X primary_email E')
+ rset.get_entity(0, 1)
+ self.assertTrue(mail.cw_relation_cached('primary_email', 'object'))
+ u = cnx.user
+ self.assertTrue(u.cw_relation_cached('primary_email', 'subject'))
+
def test_get_entity_cache_with_left_outer_join(self):
with self.admin_access.web_request() as req: