Bring back the separate web-side entity cache
authorJulien Cristau <julien.cristau@logilab.fr>
Thu, 10 Dec 2015 16:58:45 +0100
changeset 10997 da712d3f0601
parent 10996 dc572d116731
child 10998 5b646ab6821b
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
devtools/testlib.py
entity.py
rset.py
server/edition.py
server/sources/storages.py
test/unittest_entity.py
test/unittest_rset.py
web/request.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
 
--- 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
 
--- 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:
--- 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)
--- 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:
--- 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()
--- 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 '
--- 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