# HG changeset patch # User Julien Cristau # Date 1449763125 -3600 # Node ID da712d3f0601e4294d895ea51dfb65f908f7d71b # Parent dc572d11673120c7cd93e877f78d3ec5ed7a1cf5 Bring back the separate web-side entity cache Prior to changeset 635cfac73d28 "[repoapi] fold ClientConnection into Connection", we had two entity caches: one on the client/dbapi/web side, and one on the server/repo side. This is a waste, but it is actually needed as long as we have the magic _cw attribute on entities which must sometimes be a request and sometimes a cnx. Removing the duplication caused weird problems with entity._cw alternating between both types of objects, which is unexpected by both the repo and the web sides. We add an entity cache on ConnectionCubicWebRequestBase, separate from the Connection's, and bring back the _cw_update_attr_cache/dont-cache-attrs mechanism, to let the server/edition code let caches know which attributes have been modified Entity.as_rset can be cached again, as ResultSet no longer modifies the entity when fetching it from a cache. Contrary to the pre-3.21 code, _cw_update_attr_cache now handles web requests and connections in the same way (otherwise the cache ends up with wrong values if a hook modifies attributes), but dont-cache-attrs is never set for (inlined) relations. Closes #6863543 diff -r dc572d116731 -r da712d3f0601 devtools/testlib.py --- a/devtools/testlib.py Wed Dec 09 18:21:55 2015 +0100 +++ b/devtools/testlib.py Thu Dec 10 16:58:45 2015 +0100 @@ -230,8 +230,6 @@ req = self.requestcls(self._repo.vreg, url=url, headers=headers, method=method, form=kwargs) with self._session.new_cnx() as cnx: - if 'ecache' in cnx.transaction_data: - del cnx.transaction_data['ecache'] req.set_cnx(cnx) yield req diff -r dc572d116731 -r da712d3f0601 entity.py --- a/entity.py Wed Dec 09 18:21:55 2015 +0100 +++ b/entity.py Thu Dec 10 16:58:45 2015 +0100 @@ -518,7 +518,7 @@ prefixing the relation name by 'reverse_'. Also, relation values may be an entity or eid, a list of entities or eids. """ - rql, qargs, pendingrels, _attrcache = cls._cw_build_entity_query(kwargs) + rql, qargs, pendingrels, attrcache = cls._cw_build_entity_query(kwargs) if rql: rql = 'INSERT %s X: %s' % (cls.__regid__, rql) else: @@ -528,6 +528,7 @@ except IndexError: raise Exception('could not create a %r with %r (%r)' % (cls.__regid__, rql, qargs)) + created._cw_update_attr_cache(attrcache) cls._cw_handle_pending_relations(created.eid, pendingrels, execute) return created @@ -562,6 +563,7 @@ def _cw_update_attr_cache(self, attrcache): trdata = self._cw.transaction_data uncached_attrs = trdata.get('%s.storage-special-process-attrs' % self.eid, set()) + uncached_attrs.update(trdata.get('%s.dont-cache-attrs' % self.eid, set())) for attr in uncached_attrs: attrcache.pop(attr, None) self.cw_attr_cache.pop(attr, None) @@ -579,7 +581,9 @@ """ trdata = self._cw.transaction_data - trdata.setdefault('%s.storage-special-process-attrs' % self.eid, set()).add(attr) + trdata.setdefault('%s.dont-cache-attrs' % self.eid, set()).add(attr) + if repo_side: + trdata.setdefault('%s.storage-special-process-attrs' % self.eid, set()).add(attr) def __json_encode__(self): """custom json dumps hook to dump the entity's eid @@ -822,6 +826,7 @@ # data fetching methods ################################################### + @cached def as_rset(self): # XXX .cw_as_rset """returns a resultset containing `self` information""" rset = ResultSet([(self.eid,)], 'Any X WHERE X eid %(x)s', @@ -1312,6 +1317,10 @@ else: rql += ' WHERE X eid %(x)s' self._cw.execute(rql, qargs) + # update current local object _after_ the rql query to avoid + # interferences between the query execution itself and the cw_edited / + # skip_security machinery + self._cw_update_attr_cache(attrcache) self._cw_handle_pending_relations(self.eid, pendingrels, self._cw.execute) # XXX update relation cache diff -r dc572d116731 -r da712d3f0601 rset.py --- a/rset.py Wed Dec 09 18:21:55 2015 +0100 +++ b/rset.py Thu Dec 10 16:58:45 2015 +0100 @@ -483,7 +483,6 @@ # new attributes found in this resultset ? try: entity = req.entity_cache(eid) - entity._cw = req except KeyError: pass else: diff -r dc572d116731 -r da712d3f0601 server/edition.py --- a/server/edition.py Wed Dec 09 18:21:55 2015 +0100 +++ b/server/edition.py Thu Dec 10 16:58:45 2015 +0100 @@ -106,6 +106,8 @@ assert not self.saved, 'too late to modify edited attributes' super(EditedEntity, self).__setitem__(attr, value) self.entity.cw_attr_cache[attr] = value + if self.entity._cw.vreg.schema.rschema(attr).final: + self.entity._cw_dont_cache_attribute(attr) def oldnewvalue(self, attr): """returns the couple (old attr value, new attr value) diff -r dc572d116731 -r da712d3f0601 server/sources/storages.py --- a/server/sources/storages.py Wed Dec 09 18:21:55 2015 +0100 +++ b/server/sources/storages.py Thu Dec 10 16:58:45 2015 +0100 @@ -154,7 +154,7 @@ """an entity using this storage for attr has been added""" if entity._cw.transaction_data.get('fs_importing'): binary = Binary.from_file(entity.cw_edited[attr].getvalue()) - entity._cw_dont_cache_attribute(attr) + entity._cw_dont_cache_attribute(attr, repo_side=True) else: binary = entity.cw_edited.pop(attr) fd, fpath = self.new_fs_path(entity, attr) @@ -174,7 +174,7 @@ # We do not need to create it but we need to fetch the content of # the file as the actual content of the attribute fpath = entity.cw_edited[attr].getvalue() - entity._cw_dont_cache_attribute(attr) + entity._cw_dont_cache_attribute(attr, repo_side=True) assert fpath is not None binary = Binary.from_file(fpath) else: diff -r dc572d116731 -r da712d3f0601 test/unittest_entity.py --- a/test/unittest_entity.py Wed Dec 09 18:21:55 2015 +0100 +++ b/test/unittest_entity.py Thu Dec 10 16:58:45 2015 +0100 @@ -142,24 +142,13 @@ with self.admin_access.web_request() as req: user = req.execute('Any X WHERE X eid %(x)s', {'x':req.user.eid}).get_entity(0, 0) adeleid = req.execute('INSERT EmailAddress X: X address "toto@logilab.org", U use_email X WHERE U login "admin"')[0][0] - self.assertEqual({}, user._cw_related_cache) req.cnx.commit() - self.assertEqual(['primary_email_subject', 'use_email_subject', 'wf_info_for_object'], - sorted(user._cw_related_cache)) + self.assertEqual(user._cw_related_cache, {}) email = user.primary_email[0] - self.assertEqual(u'toto@logilab.org', email.address) - self.assertEqual(['created_by_subject', - 'cw_source_subject', - 'is_instance_of_subject', - 'is_subject', - 'owned_by_subject', - 'prefered_form_object', - 'prefered_form_subject', - 'primary_email_object', - 'use_email_object'], - sorted(email._cw_related_cache)) - self.assertEqual('admin', email._cw_related_cache['primary_email_object'][1][0].login) + self.assertEqual(sorted(user._cw_related_cache), ['primary_email_subject']) + self.assertEqual(list(email._cw_related_cache), ['primary_email_object']) groups = user.in_group + self.assertEqual(sorted(user._cw_related_cache), ['in_group_subject', 'primary_email_subject']) for group in groups: self.assertNotIn('in_group_subject', group._cw_related_cache) user.cw_clear_all_caches() diff -r dc572d116731 -r da712d3f0601 test/unittest_rset.py --- a/test/unittest_rset.py Wed Dec 09 18:21:55 2015 +0100 +++ b/test/unittest_rset.py Thu Dec 10 16:58:45 2015 +0100 @@ -285,7 +285,7 @@ with self.admin_access.web_request() as req: req.create_entity('CWUser', login=u'adim', upassword='adim', surname=u'di mascio', firstname=u'adrien') - req.cnx.drop_entity_cache() + req.drop_entity_cache() e = req.execute('Any X,T WHERE X login "adim", X surname T').get_entity(0, 0) self.assertEqual(e.cw_attr_cache['surname'], 'di mascio') self.assertRaises(KeyError, e.cw_attr_cache.__getitem__, 'firstname') @@ -298,7 +298,7 @@ def test_get_entity_advanced(self): with self.admin_access.web_request() as req: req.create_entity('Bookmark', title=u'zou', path=u'/view') - req.cnx.drop_entity_cache() + req.drop_entity_cache() req.execute('SET X bookmarked_by Y WHERE X is Bookmark, Y login "anon"') rset = req.execute('Any X,Y,XT,YN WHERE X bookmarked_by Y, X title XT, Y login YN') @@ -345,7 +345,7 @@ e = rset.get_entity(0, 0) self.assertEqual(e.cw_attr_cache['title'], 'zou') self.assertEqual(pprelcachedict(e._cw_related_cache), - [('created_by_subject', [req.user.eid])]) + [('created_by_subject', [req.user.eid])]) # first level of recursion u = e.created_by[0] self.assertEqual(u.cw_attr_cache['login'], 'admin') @@ -374,7 +374,7 @@ def test_get_entity_union(self): with self.admin_access.web_request() as req: e = req.create_entity('Bookmark', title=u'manger', path=u'path') - req.cnx.drop_entity_cache() + req.drop_entity_cache() rset = req.execute('Any X,N ORDERBY N WITH X,N BEING ' '((Any X,N WHERE X is Bookmark, X title N)' ' UNION ' diff -r dc572d116731 -r da712d3f0601 web/request.py --- a/web/request.py Wed Dec 09 18:21:55 2015 +0100 +++ b/web/request.py Thu Dec 10 16:58:45 2015 +0100 @@ -980,8 +980,6 @@ return self.cnx.transaction_data def set_cnx(self, cnx): - if 'ecache' in cnx.transaction_data: - del cnx.transaction_data['ecache'] self.cnx = cnx self.session = cnx.session self._set_user(cnx.user) @@ -1021,12 +1019,21 @@ # entities cache management ############################################### - entity_cache = _cnx_func('entity_cache') - set_entity_cache = _cnx_func('set_entity_cache') - cached_entities = _cnx_func('cached_entities') - drop_entity_cache = _cnx_func('drop_entity_cache') + def entity_cache(self, eid): + return self.transaction_data['req_ecache'][eid] + + def set_entity_cache(self, entity): + ecache = self.transaction_data.setdefault('req_ecache', {}) + ecache.setdefault(entity.eid, entity) + def cached_entities(self): + return self.transaction_data.get('req_ecache', {}).values() + def drop_entity_cache(self, eid=None): + if eid is None: + self.transaction_data.pop('req_ecache', None) + else: + del self.transaction_data['req_ecache'][eid] CubicWebRequestBase = ConnectionCubicWebRequestBase