backport stable
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Wed, 28 Jul 2010 16:31:32 +0200
changeset 6033 09db6f4619c9
parent 6032 52f9a43d8e08 (diff)
parent 6031 4b721e739f53 (current diff)
child 6039 6e84db1b3e44
backport stable
server/session.py
web/views/editcontroller.py
--- a/dbapi.py	Wed Jul 28 16:27:57 2010 +0200
+++ b/dbapi.py	Wed Jul 28 16:31:32 2010 +0200
@@ -313,19 +313,17 @@
 
     # low level session data management #######################################
 
-    def get_shared_data(self, key, default=None, pop=False):
-        """return value associated to `key` in shared data"""
-        return self.cnx.get_shared_data(key, default, pop)
-
-    def set_shared_data(self, key, value, querydata=False):
-        """set value associated to `key` in shared data
+    def get_shared_data(self, key, default=None, pop=False, txdata=False):
+        """see :meth:`Connection.get_shared_data`"""
+        return self.cnx.get_shared_data(key, default, pop, txdata)
 
-        if `querydata` is true, the value will be added to the repository
-        session's query data which are cleared on commit/rollback of the current
-        transaction, and won't be available through the connexion, only on the
-        repository side.
-        """
-        return self.cnx.set_shared_data(key, value, querydata)
+    def set_shared_data(self, key, value, txdata=False, querydata=None):
+        """see :meth:`Connection.set_shared_data`"""
+        if querydata is not None:
+            txdata = querydata
+            warn('[3.10] querydata argument has been renamed to txdata',
+                 DeprecationWarning, stacklevel=2)
+        return self.cnx.set_shared_data(key, value, txdata)
 
     # server session compat layer #############################################
 
@@ -507,10 +505,12 @@
         return DBAPIRequest(self.vreg, DBAPISession(self))
 
     def check(self):
-        """raise `BadConnectionId` if the connection is no more valid"""
+        """raise `BadConnectionId` if the connection is no more valid, else
+        return its latest activity timestamp.
+        """
         if self._closed is not None:
             raise ProgrammingError('Closed connection')
-        self._repo.check_session(self.sessionid)
+        return self._repo.check_session(self.sessionid)
 
     def set_session_props(self, **props):
         """raise `BadConnectionId` if the connection is no more valid"""
@@ -518,23 +518,29 @@
             raise ProgrammingError('Closed connection')
         self._repo.set_session_props(self.sessionid, props)
 
-    def get_shared_data(self, key, default=None, pop=False):
-        """return value associated to `key` in shared data"""
-        if self._closed is not None:
-            raise ProgrammingError('Closed connection')
-        return self._repo.get_shared_data(self.sessionid, key, default, pop)
+    def get_shared_data(self, key, default=None, pop=False, txdata=False):
+        """return value associated to key in the session's data dictionary or
+        session's transaction's data if `txdata` is true.
 
-    def set_shared_data(self, key, value, querydata=False):
-        """set value associated to `key` in shared data
+        If pop is True, value will be removed from the dictionnary.
 
-        if `querydata` is true, the value will be added to the repository
-        session's query data which are cleared on commit/rollback of the current
-        transaction, and won't be available through the connexion, only on the
-        repository side.
+        If key isn't defined in the dictionnary, value specified by the
+        `default` argument will be returned.
         """
         if self._closed is not None:
             raise ProgrammingError('Closed connection')
-        return self._repo.set_shared_data(self.sessionid, key, value, querydata)
+        return self._repo.get_shared_data(self.sessionid, key, default, pop, txdata)
+
+    def set_shared_data(self, key, value, txdata=False):
+        """set value associated to `key` in shared data
+
+        if `txdata` is true, the value will be added to the repository session's
+        transaction's data which are cleared on commit/rollback of the current
+        transaction.
+        """
+        if self._closed is not None:
+            raise ProgrammingError('Closed connection')
+        return self._repo.set_shared_data(self.sessionid, key, value, txdata)
 
     def get_schema(self):
         """Return the schema currently used by the repository.
--- a/etwist/twconfig.py	Wed Jul 28 16:27:57 2010 +0200
+++ b/etwist/twconfig.py	Wed Jul 28 16:31:32 2010 +0200
@@ -76,12 +76,6 @@
 the repository rather than the user running the command',
           'group': 'main', 'level': WebConfiguration.mode == 'system'
           }),
-        ('session-time',
-         {'type' : 'time',
-          'default': '30min',
-          'help': 'session expiration time, default to 30 minutes',
-          'group': 'main', 'level': 1,
-          }),
         ('pyro-server',
          {'type' : 'yn',
           # pyro is only a recommends by default, so don't activate it here
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/misc/migration/3.10.0_common.py	Wed Jul 28 16:31:32 2010 +0200
@@ -0,0 +1,1 @@
+option_group_changed('cleanup-session-time', 'web', 'main')
--- a/server/repository.py	Wed Jul 28 16:27:57 2010 +0200
+++ b/server/repository.py	Wed Jul 28 16:31:32 2010 +0200
@@ -267,7 +267,10 @@
             # call instance level initialisation hooks
             self.hm.call_hooks('server_startup', repo=self)
             # register a task to cleanup expired session
-            self.looping_task(self.config['session-time']/3., self.clean_sessions)
+            self.cleanup_session_time = self.config['cleanup-session-time'] or 60 * 60 * 24
+            assert self.cleanup_session_time > 0
+            cleanup_session_interval = min(60*60, self.cleanup_session_time / 3)
+            self.looping_task(cleanup_session_interval, self.clean_sessions)
         assert isinstance(self._looping_tasks, list), 'already started'
         for i, (interval, func, args) in enumerate(self._looping_tasks):
             self._looping_tasks[i] = task = utils.LoopTask(interval, func, args)
@@ -622,24 +625,32 @@
             session.reset_pool()
 
     def check_session(self, sessionid):
-        """raise `BadConnectionId` if the connection is no more valid"""
-        self._get_session(sessionid, setpool=False)
+        """raise `BadConnectionId` if the connection is no more valid, else
+        return its latest activity timestamp.
+        """
+        return self._get_session(sessionid, setpool=False).timestamp
+
+    def get_shared_data(self, sessionid, key, default=None, pop=False, txdata=False):
+        """return value associated to key in the session's data dictionary or
+        session's transaction's data if `txdata` is true.
 
-    def get_shared_data(self, sessionid, key, default=None, pop=False):
-        """return the session's data dictionary"""
+        If pop is True, value will be removed from the dictionnary.
+
+        If key isn't defined in the dictionnary, value specified by the
+        `default` argument will be returned.
+        """
         session = self._get_session(sessionid, setpool=False)
-        return session.get_shared_data(key, default, pop)
+        return session.get_shared_data(key, default, pop, txdata)
 
-    def set_shared_data(self, sessionid, key, value, querydata=False):
+    def set_shared_data(self, sessionid, key, value, txdata=False):
         """set value associated to `key` in shared data
 
-        if `querydata` is true, the value will be added to the repository
-        session's query data which are cleared on commit/rollback of the current
-        transaction, and won't be available through the connexion, only on the
-        repository side.
+        if `txdata` is true, the value will be added to the repository session's
+        transaction's data which are cleared on commit/rollback of the current
+        transaction.
         """
         session = self._get_session(sessionid, setpool=False)
-        session.set_shared_data(key, value, querydata)
+        session.set_shared_data(key, value, txdata)
 
     def commit(self, sessionid, txid=None):
         """commit transaction for the session with the given id"""
@@ -771,7 +782,7 @@
         """close sessions not used since an amount of time specified in the
         configuration
         """
-        mintime = time() - self.config['session-time']
+        mintime = time() - self.cleanup_session_time
         self.debug('cleaning session unused since %s',
                    strftime('%T', localtime(mintime)))
         nbclosed = 0
--- a/server/serverconfig.py	Wed Jul 28 16:27:57 2010 +0200
+++ b/server/serverconfig.py	Wed Jul 28 16:31:32 2010 +0200
@@ -120,10 +120,16 @@
 the repository rather than the user running the command',
           'group': 'main', 'level': (CubicWebConfiguration.mode == 'installed') and 0 or 1,
           }),
-        ('session-time',
+        ('cleanup-session-time',
          {'type' : 'time',
-          'default': '30min',
-          'help': 'session expiration time, default to 30 minutes',
+          'default': '24h',
+          'help': 'duration of inactivity after which a session '
+          'will be closed, to limit memory consumption (avoid sessions that '
+          'never expire and cause memory leak when http-session-time is 0, or '
+          'because of bad client that never closes their connection). '
+          'So notice that even if http-session-time is 0 and the user don\'t '
+          'close his browser, he will have to reauthenticate after this time '
+          'of inactivity. Default to 24h.',
           'group': 'main', 'level': 3,
           }),
         ('connections-pool-size',
--- a/server/session.py	Wed Jul 28 16:27:57 2010 +0200
+++ b/server/session.py	Wed Jul 28 16:31:32 2010 +0200
@@ -618,16 +618,20 @@
 
     # shared data handling ###################################################
 
-    def get_shared_data(self, key, default=None, pop=False):
+    def get_shared_data(self, key, default=None, pop=False, txdata=False):
         """return value associated to `key` in session data"""
-        if pop:
-            return self.data.pop(key, default)
+        if txdata:
+            data = self.transaction_data
         else:
-            return self.data.get(key, default)
+            data = self.data
+        if pop:
+            return data.pop(key, default)
+        else:
+            return data.get(key, default)
 
-    def set_shared_data(self, key, value, querydata=False):
+    def set_shared_data(self, key, value, txdata=False):
         """set value associated to `key` in session data"""
-        if querydata:
+        if txdata:
             self.transaction_data[key] = value
         else:
             self.data[key] = value
--- a/server/test/unittest_repository.py	Wed Jul 28 16:27:57 2010 +0200
+++ b/server/test/unittest_repository.py	Wed Jul 28 16:31:32 2010 +0200
@@ -161,7 +161,7 @@
     def test_check_session(self):
         repo = self.repo
         cnxid = repo.connect(self.admlogin, password=self.admpassword)
-        self.assertEquals(repo.check_session(cnxid), None)
+        self.assertIsInstance(repo.check_session(cnxid), float)
         repo.close(cnxid)
         self.assertRaises(BadConnectionId, repo.check_session, cnxid)
 
--- a/web/application.py	Wed Jul 28 16:27:57 2010 +0200
+++ b/web/application.py	Wed Jul 28 16:31:32 2010 +0200
@@ -31,7 +31,7 @@
 from cubicweb import set_log_methods, cwvreg
 from cubicweb import (
     ValidationError, Unauthorized, AuthenticationError, NoSelectableObject,
-    RepositoryError, CW_EVENT_MANAGER)
+    RepositoryError, BadConnectionId, CW_EVENT_MANAGER)
 from cubicweb.dbapi import DBAPISession
 from cubicweb.web import LOGGER, component
 from cubicweb.web import (
@@ -48,48 +48,43 @@
 
     def __init__(self, vreg):
         self.session_time = vreg.config['http-session-time'] or None
-        if self.session_time is not None:
-            assert self.session_time > 0
-            self.cleanup_session_time = self.session_time
-        else:
-            self.cleanup_session_time = vreg.config['cleanup-session-time'] or 1440 * 60
-            assert self.cleanup_session_time > 0
-        self.cleanup_anon_session_time = vreg.config['cleanup-anonymous-session-time'] or 5 * 60
-        assert self.cleanup_anon_session_time > 0
         self.authmanager = vreg['components'].select('authmanager', vreg=vreg)
+        interval = (self.session_time or 0) / 2.
         if vreg.config.anonymous_user() is not None:
-            self.clean_sessions_interval = max(
-                5 * 60, min(self.cleanup_session_time / 2.,
-                            self.cleanup_anon_session_time / 2.))
-        else:
-            self.clean_sessions_interval = max(
-                5 * 60,
-                self.cleanup_session_time / 2.)
+            self.cleanup_anon_session_time = vreg.config['cleanup-anonymous-session-time'] or 5 * 60
+            assert self.cleanup_anon_session_time > 0
+            if self.session_time is not None:
+                self.cleanup_anon_session_time = min(self.session_time,
+                                                     self.cleanup_anon_session_time)
+            interval = self.cleanup_anon_session_time / 2.
+        # we don't want to check session more than once every 5 minutes
+        self.clean_sessions_interval = max(5 * 60, interval)
 
     def clean_sessions(self):
         """cleanup sessions which has not been unused since a given amount of
         time. Return the number of sessions which have been closed.
         """
         self.debug('cleaning http sessions')
+        session_time = self.session_time
         closed, total = 0, 0
         for session in self.current_sessions():
-            no_use_time = (time() - session.last_usage_time)
             total += 1
-            if session.anonymous_session:
-                if no_use_time >= self.cleanup_anon_session_time:
+            try:
+                last_usage_time = session.cnx.check()
+            except BadConnectionId:
+                self.close_session(session)
+                closed += 1
+            else:
+                no_use_time = (time() - last_usage_time)
+                if session.anonymous_session:
+                    if no_use_time >= self.cleanup_anon_session_time:
+                        self.close_session(session)
+                        closed += 1
+                elif session_time is not None and no_use_time >= session_time:
                     self.close_session(session)
                     closed += 1
-            elif no_use_time >= self.cleanup_session_time:
-                self.close_session(session)
-                closed += 1
         return closed, total - closed
 
-    def has_expired(self, session):
-        """return True if the web session associated to the session is expired
-        """
-        return not (self.session_time is None or
-                    time() < session.last_usage_time + self.session_time)
-
     def current_sessions(self):
         """return currently open sessions"""
         raise NotImplementedError()
@@ -213,8 +208,6 @@
                 except AuthenticationError:
                     req.remove_cookie(cookie, self.SESSION_VAR)
                     raise
-        # remember last usage time for web session tracking
-        session.last_usage_time = time()
 
     def get_session(self, req, sessionid):
         return self.session_manager.get_session(req, sessionid)
@@ -224,8 +217,6 @@
         cookie = req.get_cookie()
         cookie[self.SESSION_VAR] = session.sessionid
         req.set_cookie(cookie, self.SESSION_VAR, maxage=None)
-        # remember last usage time for web session tracking
-        session.last_usage_time = time()
         if not session.anonymous_session:
             self._postlogin(req)
         return session
--- a/web/test/unittest_session.py	Wed Jul 28 16:27:57 2010 +0200
+++ b/web/test/unittest_session.py	Wed Jul 28 16:31:32 2010 +0200
@@ -7,10 +7,11 @@
 :license: GNU Lesser General Public License, v2.1 - http://www.gnu.org/licenses
 """
 from cubicweb.devtools.testlib import CubicWebTC
+from cubicweb.web import InvalidSession
 
 class SessionTC(CubicWebTC):
 
-    def test_auto_reconnection(self):
+    def test_session_expiration(self):
         sm = self.app.session_handler.session_manager
         # make is if the web session has been opened by the session manager
         sm._sessions[self.cnx.sessionid] = self.websession
@@ -23,11 +24,8 @@
             # fake an incoming http query with sessionid in session cookie
             # don't use self.request() which try to call req.set_session
             req = self.requestcls(self.vreg)
-            websession = sm.get_session(req, sessionid)
-            self.assertEquals(len(sm._sessions), 1)
-            self.assertIs(websession, self.websession)
-            self.assertEquals(websession.sessionid, sessionid)
-            self.assertNotEquals(websession.sessionid, websession.cnx.sessionid)
+            self.assertRaises(InvalidSession, sm.get_session, req, sessionid)
+            self.assertEquals(len(sm._sessions), 0)
         finally:
             # avoid error in tearDown by telling this connection is closed...
             self.cnx._closed = True
--- a/web/views/authentication.py	Wed Jul 28 16:27:57 2010 +0200
+++ b/web/views/authentication.py	Wed Jul 28 16:31:32 2010 +0200
@@ -74,7 +74,7 @@
         self.repo = vreg.config.repository(vreg)
         self.log_queries = vreg.config['query-log-file']
         self.authinforetreivers = sorted(vreg['webauth'].possible_objects(vreg),
-                                    key=lambda x: x.order)
+                                         key=lambda x: x.order)
         # 2-uple login / password, login is None when no anonymous access
         # configured
         self.anoninfo = vreg.config.anonymous_user()
@@ -98,25 +98,11 @@
         if login and session.login != login:
             raise InvalidSession('login mismatch')
         try:
-            lock = session.reconnection_lock
-        except AttributeError:
-            lock = session.reconnection_lock = Lock()
-        # need to be locked two avoid duplicated reconnections on concurrent
-        # requests
-        with lock:
-            cnx = session.cnx
-            try:
-                # calling cnx.user() check connection validity, raise
-                # BadConnectionId on failure
-                user = cnx.user(req)
-            except BadConnectionId:
-                # check if a connection should be automatically restablished
-                if (login is None or login == session.login):
-                    cnx = self._authenticate(session.login, session.authinfo)
-                    user = cnx.user(req)
-                    session.cnx = cnx
-                else:
-                    raise InvalidSession('bad connection id')
+            # calling cnx.user() check connection validity, raise
+            # BadConnectionId on failure
+            user = session.cnx.user(req)
+        except BadConnectionId:
+            raise InvalidSession('bad connection id')
         return user
 
     def authenticate(self, req):
--- a/web/views/debug.py	Wed Jul 28 16:27:57 2010 +0200
+++ b/web/views/debug.py	Wed Jul 28 16:31:32 2010 +0200
@@ -119,10 +119,15 @@
             if sessions:
                 w(u'<ul>')
                 for session in sessions:
+                    try:
+                        last_usage_time = session.cnx.check()
+                    except BadConnectionId:
+                        w(u'<li>%s (INVALID)</li>' % session.sessionid)
+                        continue
                     w(u'<li>%s (%s: %s)<br/>' % (
                         session.sessionid,
                         _('last usage'),
-                        strftime(dtformat, localtime(session.last_usage_time))))
+                        strftime(dtformat, localtime(last_usage_time))))
                     dict_to_html(w, session.data)
                     w(u'</li>')
                 w(u'</ul>')
--- a/web/views/editcontroller.py	Wed Jul 28 16:27:57 2010 +0200
+++ b/web/views/editcontroller.py	Wed Jul 28 16:31:32 2010 +0200
@@ -115,7 +115,7 @@
         form = req.form
         # so we're able to know the main entity from the repository side
         if '__maineid' in form:
-            req.set_shared_data('__maineid', form['__maineid'], querydata=True)
+            req.set_shared_data('__maineid', form['__maineid'], txdata=True)
         # no specific action, generic edition
         self._to_create = req.data['eidmap'] = {}
         self._pending_fields = req.data['pendingfields'] = set()
--- a/web/views/sessions.py	Wed Jul 28 16:27:57 2010 +0200
+++ b/web/views/sessions.py	Wed Jul 28 16:31:32 2010 +0200
@@ -17,8 +17,8 @@
 # with CubicWeb.  If not, see <http://www.gnu.org/licenses/>.
 """web session component: by dfault the session is actually the db connection
 object :/
+"""
 
-"""
 __docformat__ = "restructuredtext en"
 
 from cubicweb.web import InvalidSession
@@ -51,9 +51,6 @@
         if not sessionid in self._sessions:
             raise InvalidSession()
         session = self._sessions[sessionid]
-        if self.has_expired(session):
-            self.close_session(session)
-            raise InvalidSession()
         try:
             user = self.authmanager.validate_session(req, session)
         except InvalidSession:
--- a/web/webconfig.py	Wed Jul 28 16:27:57 2010 +0200
+++ b/web/webconfig.py	Wed Jul 28 16:31:32 2010 +0200
@@ -135,17 +135,6 @@
           "Should be 0 or greater than repository\'s session-time.",
           'group': 'web', 'level': 2,
           }),
-        ('cleanup-session-time',
-         {'type' : 'time',
-          'default': '24h',
-          'help': 'duration of inactivity after which a connection '
-          'will be closed, to limit memory consumption (avoid sessions that '
-          'never expire and cause memory leak when http-session-time is 0). '
-          'So even if http-session-time is 0 and the user don\'t close his '
-          'browser, he will have to reauthenticate after this time of '
-          'inactivity. Default to 24h.',
-          'group': 'web', 'level': 3,
-          }),
         ('cleanup-anonymous-session-time',
          {'type' : 'time',
           'default': '5min',