# HG changeset patch # User Aurelien Campeas # Date 1363702702 -3600 # Node ID 5567a5117aeb5656c4bc1df5b85d5f8b461c54ac # Parent 3530b7494195c11af5f5c1f36177574c44770045 [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 ... diff -r 3530b7494195 -r 5567a5117aeb entity.py --- a/entity.py Tue Mar 19 15:17:34 2013 +0100 +++ b/entity.py Tue Mar 19 15:18:22 2013 +0100 @@ -997,30 +997,38 @@ :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 @@ # 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 @@ 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 @@ 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 @@ """ 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: diff -r 3530b7494195 -r 5567a5117aeb test/unittest_entity.py --- a/test/unittest_entity.py Tue Mar 19 15:17:34 2013 +0100 +++ b/test/unittest_entity.py Tue Mar 19 15:18:22 2013 +0100 @@ -141,6 +141,9 @@ 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() diff -r 3530b7494195 -r 5567a5117aeb test/unittest_rset.py --- a/test/unittest_rset.py Tue Mar 19 15:17:34 2013 +0100 +++ b/test/unittest_rset.py Tue Mar 19 15:18:22 2013 +0100 @@ -345,9 +345,9 @@ 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)