[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.
--- 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()
--- 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)
--- 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.
--- 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
--- 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
--- 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):
--- 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 <http://www.gnu.org/licenses/>.
"""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)
--- 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:
--- 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)
--- 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()