[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).
--- 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()