Commit 093f4f63 authored by Aurelien Campeas's avatar Aurelien Campeas
Browse files

[entity] ensure the .related(entities=False) parameter is honored all the way...

[entity] ensure the .related(entities=False) parameter is honored all the way down (closes #2755994)

As of today, such a call will always fill the relation cache by
calling .entities() on every single related rset entry.


As a consequence, the `limit` parameter handling also had to be fixed.
It was bogus in the following ways:

* not used in the related_rql, hence potentially huge database
  requests, but also actually

* foolishly used in the .entities()-calling cache routine we now
  bypass (this changeset ticket's main topic)


Now:

* we set a limit on the rql expression, and

* forbid caching if given a non-None limit (as we don't want to make
  the cache handling code more complicated than it is already)


With this, entity.unrelated gets a better limit implementation (so the
code in related/unrelated is nice and symmetric)


Risk:

* _cw_relation_cache disappears completely, which is good, but this is
  Python, so you never know ...
parent 3530b7494195
......@@ -997,30 +997,38 @@ class Entity(AppObject):
:exc:`Unauthorized`, else (the default), the exception is propagated
"""
rtype = str(rtype)
try:
return self._cw_relation_cache(rtype, role, entities, limit)
except KeyError:
pass
if limit is None:
# we cannot do much wrt cache on limited queries
cache_key = '%s_%s' % (rtype, role)
if cache_key in self._cw_related_cache:
return self._cw_related_cache[cache_key][entities]
if not self.has_eid():
if entities:
return []
return self._cw.empty_rset()
rql = self.cw_related_rql(rtype, role)
rql = self.cw_related_rql(rtype, role, limit=limit)
try:
rset = self._cw.execute(rql, {'x': self.eid})
except Unauthorized:
if not safe:
raise
rset = self._cw.empty_rset()
self.cw_set_relation_cache(rtype, role, rset)
return self.related(rtype, role, limit, entities)
if entities:
if limit is None:
self.cw_set_relation_cache(rtype, role, rset)
return self.related(rtype, role, limit, entities)
return list(rset.entities())
else:
return rset
def cw_related_rql(self, rtype, role='subject', targettypes=None):
def cw_related_rql(self, rtype, role='subject', targettypes=None, limit=None):
vreg = self._cw.vreg
rschema = vreg.schema[rtype]
select = Select()
mainvar, evar = select.get_variable('X'), select.get_variable('E')
select.add_selected(mainvar)
if limit is not None:
select.set_limit(limit)
select.add_eid_restriction(evar, 'x', 'Substitute')
if role == 'subject':
rel = make_relation(evar, rtype, (mainvar,), VariableRef)
......@@ -1075,7 +1083,7 @@ class Entity(AppObject):
# generic vocabulary methods ##############################################
def cw_unrelated_rql(self, rtype, targettype, role, ordermethod=None,
vocabconstraints=True, lt_infos={}):
vocabconstraints=True, lt_infos={}, limit=None):
"""build a rql to fetch `targettype` entities unrelated to this entity
using (rtype, role) relation.
......@@ -1104,6 +1112,8 @@ class Entity(AppObject):
searchedvar = subjvar = select.get_variable('S')
evar = objvar = select.get_variable('O')
select.add_selected(searchedvar)
if limit is not None:
select.set_limit(limit)
# initialize some variables according to `self` existence
if rdef.role_cardinality(neg_role(role)) in '?1':
# if cardinality in '1?', we want a target entity which isn't
......@@ -1197,14 +1207,10 @@ class Entity(AppObject):
by a given relation, with self as subject or object
"""
try:
rql, args = self.cw_unrelated_rql(rtype, targettype, role,
ordermethod, lt_infos=lt_infos)
rql, args = self.cw_unrelated_rql(rtype, targettype, role, limit=limit,
ordermethod=ordermethod, lt_infos=lt_infos)
except Unauthorized:
return self._cw.empty_rset()
# XXX should be set in unrelated rql when manipulating the AST
if limit is not None:
before, after = rql.split(' WHERE ', 1)
rql = '%s LIMIT %s WHERE %s' % (before, limit, after)
return self._cw.execute(rql, args)
# relations cache handling #################################################
......@@ -1215,18 +1221,6 @@ class Entity(AppObject):
"""
return self._cw_related_cache.get('%s_%s' % (rtype, role))
def _cw_relation_cache(self, rtype, role, entities=True, limit=None):
"""return values for the given relation if it's cached on the instance,
else raise `KeyError`
"""
res = self._cw_related_cache['%s_%s' % (rtype, role)][entities]
if limit is not None and limit < len(res):
if entities:
res = res[:limit]
else:
res = res.limit(limit)
return res
def cw_set_relation_cache(self, rtype, role, rset):
"""set cached values for the given relation"""
if rset:
......
......@@ -141,6 +141,9 @@ class EntityTC(CubicWebTC):
self.execute('SET X tags Y WHERE X is Tag, Y is Personne')
self.assertEqual(len(p.related('tags', 'object', limit=2)), 2)
self.assertEqual(len(p.related('tags', 'object')), 4)
p.cw_clear_all_caches()
self.assertEqual(len(p.related('tags', 'object', entities=True, limit=2)), 2)
self.assertEqual(len(p.related('tags', 'object', entities=True)), 4)
def test_cw_instantiate_relation(self):
req = self.request()
......
......@@ -345,9 +345,9 @@ class ResultSetTC(CubicWebTC):
e = rset.get_entity(0, 0)
# if any of the assertion below fails with a KeyError, the relation is not cached
# related entities should be an empty list
self.assertEqual(e._cw_relation_cache('primary_email', 'subject', True), ())
self.assertEqual(e._cw_related_cache['primary_email_subject'][True], ())
# related rset should be an empty rset
cached = e._cw_relation_cache('primary_email', 'subject', False)
cached = e._cw_related_cache['primary_email_subject'][False]
self.assertIsInstance(cached, ResultSet)
self.assertEqual(cached.rowcount, 0)
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment