# HG changeset patch # User Julien Cristau # Date 1456745572 -3600 # Node ID b81e543e623a51bc1718b0170b9d99af62259fe8 # Parent d034791621ad1b2cb46600e777fde52796d85b7f [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. diff -r d034791621ad -r b81e543e623a cubicweb/rset.py --- 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 diff -r d034791621ad -r b81e543e623a cubicweb/test/unittest_rset.py --- 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: