[rset] fix infinite recursion introduced in c1eb5a676c80
authorJulien Cristau <julien.cristau@logilab.fr>
Mon, 29 Feb 2016 12:32:52 +0100
changeset 11171 b81e543e623a
parent 11170 d034791621ad
child 11173 9441ebb30dd6
[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.
cubicweb/rset.py
cubicweb/test/unittest_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
 
--- 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: