# HG changeset patch # User Sylvain Thénault # Date 1489166680 -3600 # Node ID c62c80f20a82b59aacd00507bcf9c0f9fdfeb402 # Parent c21b399c92698f99cfdd6f104122b662b39104bb [repo] Stop closing session The only point in "closing" session is to call the `session_close` event. Since I'm not aware of any application relying on it, I think this is too costly (in term of code) to maintain and propose to drop it. diff -r c21b399c9269 -r c62c80f20a82 cubicweb/devtools/__init__.py --- a/cubicweb/devtools/__init__.py Fri Mar 10 12:07:29 2017 +0100 +++ b/cubicweb/devtools/__init__.py Fri Mar 10 18:24:40 2017 +0100 @@ -108,7 +108,6 @@ * system source is shutdown """ if not repo._needs_refresh: - repo.close_sessions() for cnxset in repo.cnxsets: cnxset.close(True) repo.system_source.shutdown() diff -r c21b399c9269 -r c62c80f20a82 cubicweb/devtools/fake.py --- a/cubicweb/devtools/fake.py Fri Mar 10 12:07:29 2017 +0100 +++ b/cubicweb/devtools/fake.py Fri Mar 10 18:24:40 2017 +0100 @@ -151,8 +151,7 @@ def commit(self, *args): self.transaction_data.clear() - def close(self, *args): - pass + def system_sql(self, sql, args=None): pass diff -r c21b399c9269 -r c62c80f20a82 cubicweb/devtools/test/unittest_testlib.py --- a/cubicweb/devtools/test/unittest_testlib.py Fri Mar 10 12:07:29 2017 +0100 +++ b/cubicweb/devtools/test/unittest_testlib.py Fri Mar 10 18:24:40 2017 +0100 @@ -285,10 +285,6 @@ self.assertTrue(rset) self.assertEqual('babar', req.form['elephant']) - def test_close(self): - acc = self.new_access('admin') - acc.close() - def test_admin_access(self): with self.admin_access.client_cnx() as cnx: self.assertEqual('admin', cnx.user.login) diff -r c21b399c9269 -r c62c80f20a82 cubicweb/devtools/testlib.py --- a/cubicweb/devtools/testlib.py Fri Mar 10 12:07:29 2017 +0100 +++ b/cubicweb/devtools/testlib.py Fri Mar 10 18:24:40 2017 +0100 @@ -223,11 +223,6 @@ .. automethod:: cubicweb.testlib.RepoAccess.cnx .. automethod:: cubicweb.testlib.RepoAccess.web_request - - The RepoAccess need to be closed to destroy the associated Session. - TestCase usually take care of this aspect for the user. - - .. automethod:: cubicweb.testlib.RepoAccess.close """ def __init__(self, repo, login, requestcls): @@ -279,10 +274,6 @@ req.set_cnx(cnx) yield req - def close(self): - """Close the session associated to the RepoAccess""" - self._session.close() - @contextmanager def shell(self): from cubicweb.server.migractions import ServerMigrationHelper @@ -360,7 +351,7 @@ def _close_access(self): while self._open_access: try: - self._open_access.pop().close() + self._open_access.pop() except BadConnectionId: continue # already closed @@ -458,7 +449,6 @@ def tearDown(self): # XXX hack until logilab.common.testlib is fixed if self._admin_session is not None: - self._admin_session.close() self._admin_session = None while self._cleanups: cleanup, args, kwargs = self._cleanups.pop(-1) diff -r c21b399c9269 -r c62c80f20a82 cubicweb/hooks/syncsession.py --- a/cubicweb/hooks/syncsession.py Fri Mar 10 12:07:29 2017 +0100 +++ b/cubicweb/hooks/syncsession.py Fri Mar 10 18:24:40 2017 +0100 @@ -116,7 +116,6 @@ # remove cached groups for the user key = user_session_cache_key(self.session.user.eid, 'groups') self.session.data.pop(key, None) - self.session.close() except BadConnectionId: pass # already closed diff -r c21b399c9269 -r c62c80f20a82 cubicweb/server/hook.py --- a/cubicweb/server/hook.py Fri Mar 10 12:07:29 2017 +0100 +++ b/cubicweb/server/hook.py Fri Mar 10 18:24:40 2017 +0100 @@ -189,8 +189,7 @@ `server_restore`) have a `repo` and a `timestamp` attributes, but *their `_cw` attribute is None*. -Hooks called on session event (eg `session_open`, `session_close`) have no -special attribute. +Hooks called on session event (`session_open`) have no special attribute. API @@ -273,7 +272,7 @@ SYSTEM_HOOKS = set(('server_backup', 'server_restore', 'server_startup', 'server_maintenance', 'server_shutdown', 'before_server_shutdown', - 'session_open', 'session_close')) + 'session_open',)) ALL_HOOKS = ENTITIES_HOOKS | RELATIONS_HOOKS | SYSTEM_HOOKS diff -r c21b399c9269 -r c62c80f20a82 cubicweb/server/repository.py --- a/cubicweb/server/repository.py Fri Mar 10 12:07:29 2017 +0100 +++ b/cubicweb/server/repository.py Fri Mar 10 18:24:40 2017 +0100 @@ -458,7 +458,6 @@ self.info('waiting thread %s...', thread.getName()) thread.join() self.info('thread %s finished', thread.getName()) - self.close_sessions() self.cnxsets.close() hits, misses = self.querier.cache_hit, self.querier.cache_miss try: @@ -691,11 +690,6 @@ # session handling ######################################################## - def close_sessions(self): - """close every opened sessions""" - for session in list(self._sessions.values()): - session.close() - def clean_sessions(self): """close sessions not used since an amount of time specified in the configuration diff -r c21b399c9269 -r c62c80f20a82 cubicweb/server/session.py --- a/cubicweb/server/session.py Fri Mar 10 12:07:29 2017 +0100 +++ b/cubicweb/server/session.py Fri Mar 10 18:24:40 2017 +0100 @@ -955,18 +955,6 @@ self.repo = repo self._timestamp = Timestamp() self.data = {} - self.closed = False - - def close(self): - if self.closed: - self.warning('closing already closed session %s', self.sessionid) - return - with self.new_cnx() as cnx: - self.repo.hm.call_hooks('session_close', cnx) - cnx.commit() - del self.repo._sessions[self.sessionid] - self.closed = True - self.info('closed session %s for user %s', self.sessionid, self.user.login) def __unicode__(self): return '' % ( diff -r c21b399c9269 -r c62c80f20a82 cubicweb/server/test/data/hooks.py --- a/cubicweb/server/test/data/hooks.py Fri Mar 10 12:07:29 2017 +0100 +++ b/cubicweb/server/test/data/hooks.py Fri Mar 10 18:24:40 2017 +0100 @@ -40,9 +40,3 @@ events = ('session_open',) def __call__(self): CALLED_EVENTS['session_open'] = self._cw.user.login - -class LogoutHook(Hook): - __regid__ = 'mylogout' - events = ('session_close',) - def __call__(self): - CALLED_EVENTS['session_close'] = self._cw.user.login diff -r c21b399c9269 -r c62c80f20a82 cubicweb/server/test/unittest_hook.py --- a/cubicweb/server/test/unittest_hook.py Fri Mar 10 12:07:29 2017 +0100 +++ b/cubicweb/server/test/unittest_hook.py Fri Mar 10 18:24:40 2017 +0100 @@ -139,8 +139,6 @@ anonaccess = self.new_access('anon') with anonaccess.repo_cnx() as cnx: self.assertEqual(hooks.CALLED_EVENTS['session_open'], 'anon') - anonaccess.close() - self.assertEqual(hooks.CALLED_EVENTS['session_close'], 'anon') if __name__ == '__main__': diff -r c21b399c9269 -r c62c80f20a82 cubicweb/server/test/unittest_ldapsource.py --- a/cubicweb/server/test/unittest_ldapsource.py Fri Mar 10 12:07:29 2017 +0100 +++ b/cubicweb/server/test/unittest_ldapsource.py Fri Mar 10 18:24:40 2017 +0100 @@ -246,9 +246,7 @@ self.assertRaises(AuthenticationError, source.authenticate, cnx, 'syt', 'toto') self.assertTrue(source.authenticate(cnx, 'syt', 'syt')) - session = self.repo.new_session('syt', password='syt') - self.assertTrue(session) - session.close() + self.assertTrue(self.repo.new_session('syt', password='syt')) def test_base(self): with self.admin_access.repo_cnx() as cnx: @@ -420,9 +418,7 @@ cnx.commit() # and that we can now authenticate again self.assertRaises(AuthenticationError, self.repo.new_session, 'syt', password='toto') - session = self.repo.new_session('syt', password='syt') - self.assertTrue(session) - session.close() + self.assertTrue(self.repo.new_session('syt', password='syt')) class LDAPFeedGroupTC(LDAPFeedTestBase): diff -r c21b399c9269 -r c62c80f20a82 cubicweb/server/test/unittest_repository.py --- a/cubicweb/server/test/unittest_repository.py Fri Mar 10 12:07:29 2017 +0100 +++ b/cubicweb/server/test/unittest_repository.py Fri Mar 10 18:24:40 2017 +0100 @@ -77,9 +77,7 @@ self.assertFalse(cnx.execute('Any X WHERE NOT X cw_source S')) def test_connect(self): - session = self.repo.new_session(self.admlogin, password=self.admpassword) - self.assertTrue(session.sessionid) - session.close() + self.assertTrue(self.repo.new_session(self.admlogin, password=self.admpassword)) self.assertRaises(AuthenticationError, self.repo.connect, self.admlogin, password='nimportnawak') self.assertRaises(AuthenticationError, @@ -100,9 +98,7 @@ {'login': u"barnabé", 'passwd': u"héhéhé".encode('UTF8')}) cnx.commit() repo = self.repo - session = repo.new_session(u"barnabé", password=u"héhéhé".encode('UTF8')) - self.assertTrue(session.sessionid) - session.close() + self.assertTrue(repo.new_session(u"barnabé", password=u"héhéhé".encode('UTF8'))) def test_rollback_on_execute_validation_error(self): class ValidationErrorAfterHook(Hook): @@ -142,14 +138,6 @@ cnx.rollback() self.assertFalse(cnx.execute('Any X WHERE X is CWGroup, X name "toto"')) - - def test_close(self): - repo = self.repo - session = repo.new_session(self.admlogin, password=self.admpassword) - self.assertTrue(session.sessionid) - session.close() - - def test_initial_schema(self): schema = self.repo.schema # check order of attributes is respected @@ -198,7 +186,6 @@ session = repo.new_session(self.admlogin, password=self.admpassword) with session.new_cnx() as cnx: self.assertEqual(repo.type_from_eid(2, cnx), 'CWGroup') - session.close() def test_public_api(self): self.assertEqual(self.repo.get_schema(), self.repo.schema) diff -r c21b399c9269 -r c62c80f20a82 cubicweb/server/test/unittest_security.py --- a/cubicweb/server/test/unittest_security.py Fri Mar 10 12:07:29 2017 +0100 +++ b/cubicweb/server/test/unittest_security.py Fri Mar 10 18:24:40 2017 +0100 @@ -86,14 +86,12 @@ "WHERE cw_login = 'oldpassword'").fetchone()[0] oldhash = self.repo.system_source.binary_to_str(oldhash) session = self.repo.new_session('oldpassword', password='oldpassword') - session.close() newhash = cnx.system_sql("SELECT cw_upassword FROM cw_CWUser " "WHERE cw_login = 'oldpassword'").fetchone()[0] newhash = self.repo.system_source.binary_to_str(newhash) self.assertNotEqual(oldhash, newhash) self.assertTrue(newhash.startswith(b'$6$')) session = self.repo.new_session('oldpassword', password='oldpassword') - session.close() newnewhash = cnx.system_sql("SELECT cw_upassword FROM cw_CWUser WHERE " "cw_login = 'oldpassword'").fetchone()[0] newnewhash = self.repo.system_source.binary_to_str(newnewhash) @@ -305,8 +303,7 @@ cnx.execute('SET X upassword %(passwd)s WHERE X eid %(x)s', {'x': ueid, 'passwd': b'newpwd'}) cnx.commit() - session = self.repo.new_session('user', password='newpwd') - session.close() + self.repo.new_session('user', password='newpwd') def test_user_cant_change_other_upassword(self): with self.admin_access.repo_cnx() as cnx: diff -r c21b399c9269 -r c62c80f20a82 cubicweb/web/views/sessions.py --- a/cubicweb/web/views/sessions.py Fri Mar 10 12:07:29 2017 +0100 +++ b/cubicweb/web/views/sessions.py Fri Mar 10 18:24:40 2017 +0100 @@ -127,9 +127,6 @@ except InvalidSession: self.close_session(session) raise - if session.closed: - self.close_session(session) - raise InvalidSession() return session def open_session(self, req): @@ -176,4 +173,3 @@ """ self.info('closing http session %s' % session.sessionid) self._sessions.pop(session.sessionid, None) - session.close()