# HG changeset patch # User Sylvain Thénault # Date 1490867882 -7200 # Node ID be8636d12afd5cfadac3ac90f3c08d2b5db36718 # Parent 1d3a9bb46339d3e9dc30f5403c3e6fd950954b13 [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). diff -r 1d3a9bb46339 -r be8636d12afd cubicweb/hooks/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) diff -r 1d3a9bb46339 -r be8636d12afd cubicweb/hooks/test/unittest_syncsession.py --- 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()