# HG changeset patch # User Aurelien Campeas # Date 1402581284 -7200 # Node ID 5de859b95988ca6ad576e9765380c53ec5ef88b7 # Parent b6b00bb1e528698ce8c82e9f9800c7f5ab9ff88d [session, repository] deprecate repo.connect and move .close reponsibility to session object Repository.new_session just returns a plain session object, and `.connect` (which returns a sessionid) is deprecated. For .close:: session.close() # done ! The session will bear the responsibility to call the "session_close" event but that's better than the previous idiom:: repo.close(session.sessionid) which involves both objects. Related to #1381328. Related to #2919309. diff -r b6b00bb1e528 -r 5de859b95988 cubicweb/devtools/__init__.py --- a/cubicweb/devtools/__init__.py Wed Mar 09 14:11:47 2016 +0100 +++ b/cubicweb/devtools/__init__.py Thu Jun 12 15:54:44 2014 +0200 @@ -106,13 +106,7 @@ * system source is shutdown """ if not repo._needs_refresh: - for sessionid in list(repo._sessions): - warnings.warn('%s Open session found while turning repository off' - %sessionid, RuntimeWarning) - try: - repo.close(sessionid) - except BadConnectionId: #this is strange ? thread issue ? - print('XXX unknown session', sessionid) + repo.close_sessions() for cnxset in repo.cnxsets: cnxset.close(True) repo.system_source.shutdown() diff -r b6b00bb1e528 -r 5de859b95988 cubicweb/devtools/testlib.py --- a/cubicweb/devtools/testlib.py Wed Mar 09 14:11:47 2016 +0100 +++ b/cubicweb/devtools/testlib.py Thu Jun 12 15:54:44 2014 +0200 @@ -267,9 +267,7 @@ def close(self): """Close the session associated to the RepoAccess""" - if self._session is not None: - self._repo.close(self._session.sessionid) - self._session = None + self._session.close() @contextmanager def shell(self): @@ -457,7 +455,7 @@ def tearDown(self): # XXX hack until logilab.common.testlib is fixed if self._admin_session is not None: - self.repo.close(self._admin_session.sessionid) + self._admin_session.close() self._admin_session = None while self._cleanups: cleanup, args, kwargs = self._cleanups.pop(-1) diff -r b6b00bb1e528 -r 5de859b95988 cubicweb/repoapi.py --- a/cubicweb/repoapi.py Wed Mar 09 14:11:47 2016 +0100 +++ b/cubicweb/repoapi.py Thu Jun 12 15:54:44 2014 +0200 @@ -47,11 +47,7 @@ """Take credential and return associated Connection. raise AuthenticationError if the credential are invalid.""" - sessionid = repo.connect(login, **kwargs) - session = repo._get_session(sessionid) - # XXX the autoclose_session should probably be handle on the session directly - # this is something to consider once we have proper server side Connection. - return Connection(session) + return repo.new_session(login, **kwargs).new_cnx() def anonymous_cnx(repo): """return a Connection for Anonymous user. diff -r b6b00bb1e528 -r 5de859b95988 cubicweb/server/repository.py --- a/cubicweb/server/repository.py Wed Mar 09 14:11:47 2016 +0100 +++ b/cubicweb/server/repository.py Thu Jun 12 15:54:44 2014 +0200 @@ -633,7 +633,7 @@ return rset.rows def new_session(self, login, **kwargs): - """open a new session for a given user + """open a *new* session for a given user raise `AuthenticationError` if the authentication failed raise `ConnectionError` if we can't open a connection @@ -655,33 +655,20 @@ cnx.commit() return session + @deprecated('[3.23] use .new_session instead (and get a plain session object)') def connect(self, login, **kwargs): - """open a new session for a given user and return its sessionid """ return self.new_session(login, **kwargs).sessionid - def close(self, sessionid, txid=None, checkshuttingdown=True): - """close the session with the given id""" - session = self._get_session(sessionid, txid=txid, - checkshuttingdown=checkshuttingdown) - # operation uncommited before close are rolled back before hook is called - with session.new_cnx() as cnx: - self.hm.call_hooks('session_close', cnx) - # commit connection at this point in case write operation has been - # done during `session_close` hooks - cnx.commit() - session.close() - del self._sessions[sessionid] - self.info('closed session %s for user %s', sessionid, session.user.login) + @deprecated('[3.23] use session.close() directly') + def close(self, sessionid): + self._get_session(sessionid).close() # session handling ######################################################## def close_sessions(self): """close every opened sessions""" - for sessionid in list(self._sessions): - try: - self.close(sessionid, checkshuttingdown=False) - except Exception: # XXX BaseException? - self.exception('error while closing session %s' % sessionid) + 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 @@ -691,9 +678,9 @@ self.debug('cleaning session unused since %s', strftime('%H:%M:%S', localtime(mintime))) nbclosed = 0 - for session in self._sessions.values(): + for session in list(self._sessions.values()): if session.timestamp < mintime: - self.close(session.sessionid) + session.close() nbclosed += 1 return nbclosed diff -r b6b00bb1e528 -r 5de859b95988 cubicweb/server/session.py --- a/cubicweb/server/session.py Wed Mar 09 14:11:47 2016 +0100 +++ b/cubicweb/server/session.py Thu Jun 12 15:54:44 2014 +0200 @@ -1017,7 +1017,15 @@ 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 __enter__(self): return self diff -r b6b00bb1e528 -r 5de859b95988 cubicweb/server/test/unittest_ldapsource.py --- a/cubicweb/server/test/unittest_ldapsource.py Wed Mar 09 14:11:47 2016 +0100 +++ b/cubicweb/server/test/unittest_ldapsource.py Thu Jun 12 15:54:44 2014 +0200 @@ -238,9 +238,9 @@ self.assertRaises(AuthenticationError, source.authenticate, cnx, 'toto', 'toto') self.assertTrue(source.authenticate(cnx, 'syt', 'syt')) - sessionid = self.repo.connect('syt', password='syt') - self.assertTrue(sessionid) - self.repo.close(sessionid) + session = self.repo.new_session('syt', password='syt') + self.assertTrue(session) + session.close() def test_base(self): with self.admin_access.repo_cnx() as cnx: @@ -321,7 +321,7 @@ cnx.commit() with self.repo.internal_cnx() as cnx: self.pull(cnx) - self.assertRaises(AuthenticationError, self.repo.connect, 'syt', password='syt') + self.assertRaises(AuthenticationError, self.repo.new_session, 'syt', password='syt') with self.admin_access.repo_cnx() as cnx: self.assertEqual(cnx.execute('Any N WHERE U login "syt", ' 'U in_state S, S name N').rows[0][0], @@ -350,7 +350,7 @@ self.delete_ldap_entry('uid=syt,ou=People,dc=cubicweb,dc=test') with self.repo.internal_cnx() as cnx: self.pull(cnx) - self.assertRaises(AuthenticationError, self.repo.connect, 'syt', password='syt') + self.assertRaises(AuthenticationError, self.repo.new_session, 'syt', password='syt') with self.admin_access.repo_cnx() as cnx: self.assertEqual(cnx.execute('Any N WHERE U login "syt", ' 'U in_state S, S name N').rows[0][0], @@ -396,16 +396,16 @@ user.cw_adapt_to('IWorkflowable').fire_transition('activate') cnx.commit() with self.assertRaises(AuthenticationError): - self.repo.connect('syt', password='syt') + self.repo.new_session('syt', password='syt') # ok now let's try to make it a system user cnx.execute('SET X cw_source S WHERE X eid %(x)s, S name "system"', {'x': user.eid}) cnx.commit() # and that we can now authenticate again - self.assertRaises(AuthenticationError, self.repo.connect, 'syt', password='toto') - sessionid = self.repo.connect('syt', password='syt') - self.assertTrue(sessionid) - self.repo.close(sessionid) + self.assertRaises(AuthenticationError, self.repo.new_session, 'syt', password='toto') + session = self.repo.new_session('syt', password='syt') + self.assertTrue(session) + session.close() class LDAPFeedGroupTC(LDAPFeedTestBase): diff -r b6b00bb1e528 -r 5de859b95988 cubicweb/server/test/unittest_repository.py --- a/cubicweb/server/test/unittest_repository.py Wed Mar 09 14:11:47 2016 +0100 +++ b/cubicweb/server/test/unittest_repository.py Thu Jun 12 15:54:44 2014 +0200 @@ -18,7 +18,6 @@ # with CubicWeb. If not, see . """unit tests for module cubicweb.server.repository""" -import threading import time import logging @@ -39,7 +38,6 @@ from cubicweb.server.sqlutils import SQL_PREFIX from cubicweb.server.hook import Hook from cubicweb.server.sources import native -from cubicweb.server.session import SessionClosedError class RepositoryTC(CubicWebTC): @@ -78,9 +76,9 @@ self.assertFalse(cnx.execute('Any X WHERE NOT X cw_source S')) def test_connect(self): - cnxid = self.repo.connect(self.admlogin, password=self.admpassword) - self.assertTrue(cnxid) - self.repo.close(cnxid) + session = self.repo.new_session(self.admlogin, password=self.admpassword) + self.assertTrue(session.sessionid) + session.close() self.assertRaises(AuthenticationError, self.repo.connect, self.admlogin, password='nimportnawak') self.assertRaises(AuthenticationError, @@ -101,9 +99,9 @@ {'login': u"barnabé", 'passwd': u"héhéhé".encode('UTF8')}) cnx.commit() repo = self.repo - cnxid = repo.connect(u"barnabé", password=u"héhéhé".encode('UTF8')) - self.assertTrue(cnxid) - repo.close(cnxid) + session = repo.new_session(u"barnabé", password=u"héhéhé".encode('UTF8')) + self.assertTrue(session.sessionid) + session.close() def test_rollback_on_execute_validation_error(self): class ValidationErrorAfterHook(Hook): @@ -146,9 +144,9 @@ def test_close(self): repo = self.repo - cnxid = repo.connect(self.admlogin, password=self.admpassword) - self.assertTrue(cnxid) - repo.close(cnxid) + session = repo.new_session(self.admlogin, password=self.admpassword) + self.assertTrue(session.sessionid) + session.close() def test_initial_schema(self): @@ -196,13 +194,12 @@ def test_internal_api(self): repo = self.repo - cnxid = repo.connect(self.admlogin, password=self.admpassword) - session = repo._get_session(cnxid) + session = repo.new_session(self.admlogin, password=self.admpassword) with session.new_cnx() as cnx: self.assertEqual(repo.type_and_source_from_eid(2, cnx), ('CWGroup', None, 'system')) self.assertEqual(repo.type_from_eid(2, cnx), 'CWGroup') - repo.close(cnxid) + session.close() def test_public_api(self): self.assertEqual(self.repo.get_schema(), self.repo.schema) diff -r b6b00bb1e528 -r 5de859b95988 cubicweb/server/test/unittest_security.py --- a/cubicweb/server/test/unittest_security.py Wed Mar 09 14:11:47 2016 +0100 +++ b/cubicweb/server/test/unittest_security.py Thu Jun 12 15:54:44 2014 +0200 @@ -84,13 +84,15 @@ oldhash = cnx.system_sql("SELECT cw_upassword FROM cw_CWUser " "WHERE cw_login = 'oldpassword'").fetchone()[0] oldhash = self.repo.system_source.binary_to_str(oldhash) - self.repo.close(self.repo.connect('oldpassword', password='oldpassword')) + 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$')) - self.repo.close(self.repo.connect('oldpassword', password='oldpassword')) + 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) @@ -300,7 +302,8 @@ cnx.execute('SET X upassword %(passwd)s WHERE X eid %(x)s', {'x': ueid, 'passwd': b'newpwd'}) cnx.commit() - self.repo.close(self.repo.connect('user', password='newpwd')) + session = self.repo.new_session('user', password='newpwd') + session.close() def test_user_cant_change_other_upassword(self): with self.admin_access.repo_cnx() as cnx: diff -r b6b00bb1e528 -r 5de859b95988 cubicweb/web/views/authentication.py --- a/cubicweb/web/views/authentication.py Wed Mar 09 14:11:47 2016 +0100 +++ b/cubicweb/web/views/authentication.py Thu Jun 12 15:54:44 2014 +0200 @@ -170,5 +170,4 @@ raise AuthenticationError() def _authenticate(self, login, authinfo): - sessionid = self.repo.connect(login, **authinfo) - return self.repo._sessions[sessionid] + return self.repo.new_session(login, **authinfo) diff -r b6b00bb1e528 -r 5de859b95988 cubicweb/web/views/sessions.py --- a/cubicweb/web/views/sessions.py Wed Mar 09 14:11:47 2016 +0100 +++ b/cubicweb/web/views/sessions.py Thu Jun 12 15:54:44 2014 +0200 @@ -176,5 +176,4 @@ """ self.info('closing http session %s' % session.sessionid) self._sessions.pop(session.sessionid, None) - if not session.closed: - session.repo.close(session.sessionid) + session.close()