[session] Drop the user session synchronization machinery 3.25
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Thu, 30 Mar 2017 11:58:02 +0200
branch3.25
changeset 12126 be8636d12afd
parent 12125 1d3a9bb46339
child 12127 078265f222e3
[session] Drop the user session synchronization machinery which should not be necessary anymore since groups and properties are fetched for each request (cache lives in transaction_data).
cubicweb/hooks/syncsession.py
cubicweb/hooks/test/unittest_syncsession.py
--- a/cubicweb/hooks/syncsession.py	Thu Mar 30 11:56:09 2017 +0200
+++ b/cubicweb/hooks/syncsession.py	Thu Mar 30 11:58:02 2017 +0200
@@ -18,36 +18,9 @@
 """Core hooks: synchronize living session on persistent data changes"""
 
 from cubicweb import _
-from cubicweb import UnknownProperty, BadConnectionId, validation_error
+from cubicweb import UnknownProperty, validation_error
 from cubicweb.predicates import is_instance
 from cubicweb.server import hook
-from cubicweb.entities.authobjs import user_session_cache_key
-
-
-def get_user_sessions(cnx, user_eid):
-    if cnx.user.eid == user_eid:
-        yield cnx
-
-
-class CachedValueMixin(object):
-    """Mixin class providing methods to retrieve some value, specified through
-    `value_name` attribute, in session data.
-    """
-    value_name = None
-    session = None  # make pylint happy
-
-    @property
-    def cached_value(self):
-        """Return cached value for the user, or None"""
-        key = user_session_cache_key(self.session.user.eid, self.value_name)
-        return self.session.data.get(key, None)
-
-    def update_cached_value(self, value):
-        """Update cached value for the user (modifying the set returned by cached_value may not be
-        necessary depending on session data implementation, e.g. redis)
-        """
-        key = user_session_cache_key(self.session.user.eid, self.value_name)
-        self.session.data[key] = value
 
 
 class SyncSessionHook(hook.Hook):
@@ -55,112 +28,6 @@
     category = 'syncsession'
 
 
-# user/groups synchronisation #################################################
-
-class _GroupOperation(CachedValueMixin, hook.Operation):
-    """Base class for group operation"""
-    value_name = 'groups'
-
-    def __init__(self, cnx, *args, **kwargs):
-        """Override to get the group name before actual groups manipulation
-
-        we may temporarily loose right access during a commit event, so
-        no query should be emitted while comitting
-        """
-        rql = 'Any N WHERE G eid %(x)s, G name N'
-        result = cnx.execute(rql, {'x': kwargs['group_eid']}, build_descr=False)
-        hook.Operation.__init__(self, cnx, *args, **kwargs)
-        self.group = result[0][0]
-
-
-class _DeleteGroupOp(_GroupOperation):
-    """Synchronize user when a in_group relation has been deleted"""
-
-    def postcommit_event(self):
-        cached_groups = self.cached_value
-        if cached_groups is not None:
-            cached_groups.remove(self.group)
-            self.update_cached_value(cached_groups)
-
-
-class _AddGroupOp(_GroupOperation):
-    """Synchronize user when a in_group relation has been added"""
-
-    def postcommit_event(self):
-        cached_groups = self.cached_value
-        if cached_groups is not None:
-            cached_groups.add(self.group)
-            self.update_cached_value(cached_groups)
-
-
-class SyncInGroupHook(SyncSessionHook):
-    """Watch addition/removal of in_group relation to synchronize living sessions accordingly"""
-    __regid__ = 'syncingroup'
-    __select__ = SyncSessionHook.__select__ & hook.match_rtype('in_group')
-    events = ('after_delete_relation', 'after_add_relation')
-
-    def __call__(self):
-        if self.event == 'after_delete_relation':
-            opcls = _DeleteGroupOp
-        else:
-            opcls = _AddGroupOp
-        for session in get_user_sessions(self._cw, self.eidfrom):
-            opcls(self._cw, session=session, group_eid=self.eidto)
-
-
-class _CloseSessionOp(hook.Operation):
-    """Close user's session when it has been deleted"""
-
-    def postcommit_event(self):
-        try:
-            # remove cached groups for the user
-            key = user_session_cache_key(self.session.user.eid, 'groups')
-            self.session.data.pop(key, None)
-        except BadConnectionId:
-            pass  # already closed
-
-
-class UserDeletedHook(SyncSessionHook):
-    """Watch deletion of user to close its opened session"""
-    __regid__ = 'closession'
-    __select__ = SyncSessionHook.__select__ & is_instance('CWUser')
-    events = ('after_delete_entity',)
-
-    def __call__(self):
-        for session in get_user_sessions(self._cw, self.entity.eid):
-            _CloseSessionOp(self._cw, session=session)
-
-
-# CWProperty hooks #############################################################
-
-
-class _UserPropertyOperation(CachedValueMixin, hook.Operation):
-    """Base class for property operation"""
-    value_name = 'properties'
-    key = None  # make pylint happy
-
-
-class _ChangeUserCWPropertyOp(_UserPropertyOperation):
-    """Synchronize cached user's properties when one has been added/updated"""
-    value = None  # make pylint happy
-
-    def postcommit_event(self):
-        cached_props = self.cached_value
-        if cached_props is not None:
-            cached_props[self.key] = self.value
-            self.update_cached_value(cached_props)
-
-
-class _DelUserCWPropertyOp(_UserPropertyOperation):
-    """Synchronize cached user's properties when one has been deleted"""
-
-    def postcommit_event(self):
-        cached_props = self.cached_value
-        if cached_props is not None:
-            cached_props.pop(self.key, None)
-            self.update_cached_value(cached_props)
-
-
 class _ChangeSiteWideCWPropertyOp(hook.Operation):
     """Synchronize site wide properties when one has been added/updated"""
     cwprop = None  # make pylint happy
@@ -223,10 +90,7 @@
             return
         except ValueError as ex:
             raise validation_error(entity, {('value', 'subject'): str(ex)})
-        if entity.for_user:
-            for session in get_user_sessions(cnx, entity.for_user[0].eid):
-                _ChangeUserCWPropertyOp(cnx, session=session, key=key, value=value)
-        else:
+        if not entity.for_user:
             _ChangeSiteWideCWPropertyOp(cnx, cwprop=self.entity)
 
 
@@ -238,7 +102,7 @@
         cnx = self._cw
         for eidfrom, rtype, eidto in cnx.transaction_data.get('pendingrelations', ()):
             if rtype == 'for_user' and eidfrom == self.entity.eid:
-                # if for_user was set, delete already handled by hook on for_user deletion
+                # not need to sync user specific properties
                 break
         else:
             _DelSiteWideCWPropertyOp(cnx, key=self.entity.pkey)
@@ -259,8 +123,6 @@
         if cnx.vreg.property_info(key)['sitewide']:
             msg = _("site-wide property can't be set for user")
             raise validation_error(eidfrom, {('for_user', 'subject'): msg})
-        for session in get_user_sessions(cnx, self.eidto):
-            _ChangeUserCWPropertyOp(cnx, session=session, key=key, value=value)
 
 
 class DelForUserRelationHook(AddForUserRelationHook):
@@ -269,8 +131,5 @@
 
     def __call__(self):
         cnx = self._cw
-        key = cnx.execute('Any K WHERE P eid %(x)s, P pkey K', {'x': self.eidfrom})[0][0]
         cnx.transaction_data.setdefault('pendingrelations', []).append(
             (self.eidfrom, self.rtype, self.eidto))
-        for session in get_user_sessions(cnx, self.eidto):
-            _DelUserCWPropertyOp(cnx, session=session, key=key)
--- a/cubicweb/hooks/test/unittest_syncsession.py	Thu Mar 30 11:56:09 2017 +0200
+++ b/cubicweb/hooks/test/unittest_syncsession.py	Thu Mar 30 11:58:02 2017 +0200
@@ -80,23 +80,6 @@
             cnx.commit()
         self.assertEqual(self.vreg.property_value('test.int'), 42)
 
-    def test_sync_user_props(self):
-        with self.admin_access.client_cnx() as cnx:
-            self.assertNotIn('ui.language', cnx.user.properties)
-            cnx.user.set_property(u'ui.language', u'fr')
-            self.assertNotIn('ui.language', cnx.user.properties)
-            cnx.commit()
-            self.assertEqual(cnx.user.properties['ui.language'], 'fr')
-            cnx.user.set_property(u'ui.language', u'en')
-            self.assertEqual(cnx.user.properties['ui.language'], 'fr')
-            cnx.commit()
-            self.assertEqual(cnx.user.properties['ui.language'], 'en')
-            cnx.execute('DELETE CWProperty X WHERE X for_user U, U eid %(u)s',
-                        {'u': cnx.user.eid})
-            self.assertEqual(cnx.user.properties['ui.language'], 'en')
-            cnx.commit()
-            self.assertNotIn('ui.language', cnx.user.properties)
-
     def test_sync_sitewide_props(self):
         with self.admin_access.client_cnx() as cnx:
             self.assertNotIn('ui.language', cnx.vreg['propertyvalues'])
@@ -114,22 +97,6 @@
             self.assertNotIn('ui.language', cnx.vreg['propertyvalues'])
 
 
-class UserGroupsSyncTC(CubicWebTC):
-
-    def test_sync_groups(self):
-        with self.admin_access.client_cnx() as cnx:
-            cnx.execute('SET U in_group G WHERE G name "users", U eid %(u)s',
-                        {'u': cnx.user.eid})
-            self.assertEqual(cnx.user.groups, set(['managers']))
-            cnx.commit()
-            self.assertEqual(cnx.user.groups, set(['managers', 'users']))
-            cnx.execute('DELETE U in_group G WHERE G name "users", U eid %(u)s',
-                        {'u': cnx.user.eid})
-            self.assertEqual(cnx.user.groups, set(['managers', 'users']))
-            cnx.commit()
-            self.assertEqual(cnx.user.groups, set(['managers']))
-
-
 if __name__ == '__main__':
     import unittest
     unittest.main()