Stop using Session on the repository side
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Tue, 14 Mar 2017 11:07:58 +0100
changeset 12043 b8d2e6b9f548
parent 12042 5e64a98572de
child 12044 70bb46dfa87b
Stop using Session on the repository side Only expect session on web request, and let the web session/authentication managers provide them. Access to cnx.data, which used to return session data, is deprecated: there is no more access to session data from the repository side, and they should be access from req.session.data from the web side.
cubicweb/devtools/testlib.py
cubicweb/pyramid/core.py
cubicweb/server/repository.py
cubicweb/server/session.py
cubicweb/sobjects/notification.py
cubicweb/test/unittest_repoapi.py
cubicweb/web/application.py
cubicweb/web/test/unittest_application.py
cubicweb/web/test/unittest_views_basecontrollers.py
cubicweb/web/views/authentication.py
--- a/cubicweb/devtools/testlib.py	Fri Mar 10 18:00:13 2017 +0100
+++ b/cubicweb/devtools/testlib.py	Tue Mar 14 11:07:58 2017 +0100
@@ -49,11 +49,10 @@
 from cubicweb.sobjects import notification
 from cubicweb.web import Redirect, application, eid_param
 from cubicweb.server.hook import SendMailOp
-from cubicweb.server.session import Session
 from cubicweb.devtools import SYSTEM_ENTITIES, SYSTEM_RELATIONS, VIEW_VALIDATORS
 from cubicweb.devtools import fake, htmlparser, DEFAULT_EMPTY_DB_ID
 from cubicweb.devtools.fill import insert_entity_queries, make_relations_queries
-
+from cubicweb.web.views.authentication import Session
 
 if sys.version_info[:2] < (3, 4):
     from unittest2 import TestCase
@@ -251,11 +250,12 @@
             req.cnx.commit()
             req.cnx.rolback()
         """
+        session = kwargs.pop('session', Session(self._repo, self._user))
         req = self.requestcls(self._repo.vreg, url=url, headers=headers,
                               method=method, form=kwargs)
         with self.cnx() as cnx:
             # web request expect a session attribute on cnx referencing the web session
-            cnx.session = Session(self._repo, self._user)
+            cnx.session = session
             req.set_cnx(cnx)
             yield req
 
@@ -681,10 +681,10 @@
         return ctrl.publish(), req
 
     @contextmanager
-    def remote_calling(self, fname, *args):
+    def remote_calling(self, fname, *args, **kwargs):
         """remote json call simulation"""
         args = [json.dumps(arg) for arg in args]
-        with self.admin_access.web_request(fname=fname, pageid='123', arg=args) as req:
+        with self.admin_access.web_request(fname=fname, pageid='123', arg=args, **kwargs) as req:
             ctrl = self.vreg['controllers'].select('ajax', req)
             yield ctrl.publish(), req
 
--- a/cubicweb/pyramid/core.py	Fri Mar 10 18:00:13 2017 +0100
+++ b/cubicweb/pyramid/core.py	Tue Mar 14 11:07:58 2017 +0100
@@ -52,12 +52,12 @@
     """
 
     def __init__(self, session, *args, **kw):
-        super(Connection, self).__init__(session, *args, **kw)
-        self._session = session
+        super(Connection, self).__init__(session._repo, session._user, *args, **kw)
+        self.session = session
         self.lang = session._cached_lang
 
     def _get_session_data(self):
-        return self._session.data
+        return self.session.data
 
     def _set_session_data(self, data):
         pass
@@ -65,15 +65,26 @@
     _session_data = property(_get_session_data, _set_session_data)
 
 
-class Session(cwsession.Session):
+class Session(object):
     """ A Session that access the session data through a property.
 
     Along with :class:`Connection`, it avoid any load of the pyramid session
     data until it is actually accessed.
     """
     def __init__(self, pyramid_request, user, repo):
-        super(Session, self).__init__(user, repo)
         self._pyramid_request = pyramid_request
+        self._user = user
+        self._repo = repo
+
+    @property
+    def anonymous_session(self):
+        # XXX for now, anonymous_user only exists in webconfig (and testconfig).
+        # It will only be present inside all-in-one instance.
+        # there is plan to move it down to global config.
+        if not hasattr(self._repo.config, 'anonymous_user'):
+            # not a web or test config, no anonymous user
+            return False
+        return self._user.login == self._repo.config.anonymous_user()[0]
 
     def get_data(self):
         if not getattr(self, '_protect_data_access', False):
--- a/cubicweb/server/repository.py	Fri Mar 10 18:00:13 2017 +0100
+++ b/cubicweb/server/repository.py	Tue Mar 14 11:07:58 2017 +0100
@@ -47,7 +47,7 @@
 from cubicweb import set_log_methods
 from cubicweb import cwvreg, schema, server
 from cubicweb.server import utils, hook, querier, sources
-from cubicweb.server.session import Session, InternalManager
+from cubicweb.server.session import InternalManager, Connection
 
 
 NO_CACHE_RELATIONS = set([
@@ -667,7 +667,7 @@
 
         Internal connections have all hooks beside security enabled.
         """
-        with Session(InternalManager(), self).new_cnx() as cnx:
+        with Connection(self, InternalManager()) as cnx:
             cnx.user._cw = cnx  # XXX remove when "vreg = user._cw.vreg" hack in entity.py is gone
             with cnx.security_enabled(read=False, write=False):
                 yield cnx
--- a/cubicweb/server/session.py	Fri Mar 10 18:00:13 2017 +0100
+++ b/cubicweb/server/session.py	Tue Mar 14 11:07:58 2017 +0100
@@ -236,23 +236,21 @@
     is_request = False
     hooks_in_progress = False
 
-    def __init__(self, session):
-        super(Connection, self).__init__(session.repo.vreg)
+    def __init__(self, repo, user):
+        super(Connection, self).__init__(repo.vreg)
         #: connection unique id
         self._open = None
-        self.session = session
 
         #: server.Repository object
-        self.repo = session.repo
+        self.repo = repo
         self.vreg = self.repo.vreg
         self._execute = self.repo.querier.execute
 
         # internal (root) session
-        self.is_internal_session = isinstance(session.user, InternalManager)
+        self.is_internal_session = isinstance(user, InternalManager)
 
         #: dict containing arbitrary data cleared at the end of the transaction
         self.transaction_data = {}
-        self._session_data = session.data
         #: ordered list of operations to be processed on commit/rollback
         self.pending_operations = []
         #: (None, 'precommit', 'postcommit', 'uncommitable')
@@ -273,7 +271,7 @@
         self.write_security = DEFAULT_SECURITY
 
         # undo control
-        config = session.repo.config
+        config = repo.config
         if config.creating or config.repairing or self.is_internal_session:
             self.undo_actions = False
         else:
@@ -283,10 +281,10 @@
         self._rewriter = RQLRewriter(self)
 
         # other session utility
-        if session.user.login == '__internal_manager__':
-            self.user = session.user
+        if user.login == '__internal_manager__':
+            self.user = user
         else:
-            self._set_user(session.user)
+            self._set_user(user)
 
     @_open_only
     def get_schema(self):
@@ -392,8 +390,9 @@
     # shared data handling ###################################################
 
     @property
+    @deprecated('[3.25] use transaction_data or req.session.data', stacklevel=3)
     def data(self):
-        return self._session_data
+        return self.transaction_data
 
     @property
     def rql_rewriter(self):
@@ -406,7 +405,7 @@
         if txdata:
             data = self.transaction_data
         else:
-            data = self._session_data
+            data = self.data
         if pop:
             return data.pop(key, default)
         else:
@@ -419,7 +418,7 @@
         if txdata:
             self.transaction_data[key] = value
         else:
-            self._session_data[key] = value
+            self.data[key] = value
 
     def clear(self):
         """reset internal data"""
@@ -450,10 +449,6 @@
     def ensure_cnx_set(self):
         yield
 
-    @property
-    def anonymous_connection(self):
-        return self.session.anonymous_session
-
     # Entity cache management #################################################
     #
     # The connection entity cache as held in cnx.transaction_data is removed at the
--- a/cubicweb/sobjects/notification.py	Fri Mar 10 18:00:13 2017 +0100
+++ b/cubicweb/sobjects/notification.py	Tue Mar 14 11:07:58 2017 +0100
@@ -32,7 +32,7 @@
 from cubicweb.view import Component, EntityView
 from cubicweb.server.hook import SendMailOp
 from cubicweb.mail import construct_message_id, format_mail
-from cubicweb.server.session import Session, InternalManager
+from cubicweb.server.session import Connection, InternalManager
 
 
 class RecipientsFinder(Component):
@@ -120,8 +120,7 @@
                 emailaddr = something.cw_adapt_to('IEmailable').get_email()
                 user = something
             # hi-jack self._cw to get a session for the returned user
-            session = Session(user, self._cw.repo)
-            with session.new_cnx() as cnx:
+            with Connection(self._cw.repo, user) as cnx:
                 self._cw = cnx
                 try:
                     # since the same view (eg self) may be called multiple time and we
--- a/cubicweb/test/unittest_repoapi.py	Fri Mar 10 18:00:13 2017 +0100
+++ b/cubicweb/test/unittest_repoapi.py	Tue Mar 14 11:07:58 2017 +0100
@@ -56,7 +56,7 @@
         """Check that ClientConnection requires explicit open and close
         """
         access = self.admin_access
-        cltcnx = Connection(access._session)
+        cltcnx = Connection(access._repo, access._user)
         # connection not open yet
         with self.assertRaises(ProgrammingError):
             cltcnx.execute('Any X WHERE X is CWUser')
--- a/cubicweb/web/application.py	Fri Mar 10 18:00:13 2017 +0100
+++ b/cubicweb/web/application.py	Tue Mar 14 11:07:58 2017 +0100
@@ -77,10 +77,14 @@
 
 @contextmanager
 def anonymized_request(req):
+    from cubicweb.web.views.authentication import Session
+
     orig_cnx = req.cnx
     anon_cnx = anonymous_cnx(orig_cnx.session.repo)
     try:
         with anon_cnx:
+            # web request expect a session attribute on cnx referencing the web session
+            anon_cnx.session = Session(orig_cnx.session.repo, anon_cnx.user)
             req.set_cnx(anon_cnx)
             yield req
     finally:
--- a/cubicweb/web/test/unittest_application.py	Fri Mar 10 18:00:13 2017 +0100
+++ b/cubicweb/web/test/unittest_application.py	Tue Mar 14 11:07:58 2017 +0100
@@ -701,7 +701,7 @@
         with cnx:
             req.set_cnx(cnx)
         self.assertEqual(len(self.open_sessions), 1)
-        self.assertEqual(asession.login, 'anon')
+        self.assertEqual(asession.user.login, 'anon')
         self.assertTrue(asession.anonymous_session)
         self._reset_cookie(req)
 
--- a/cubicweb/web/test/unittest_views_basecontrollers.py	Fri Mar 10 18:00:13 2017 +0100
+++ b/cubicweb/web/test/unittest_views_basecontrollers.py	Tue Mar 14 11:07:58 2017 +0100
@@ -884,7 +884,8 @@
                 (self.schema['tags'].rdefs['Tag', 'CWUser'],
                  {'delete': (RRQLExpression('S owned_by U'), )}, )):
             with self.admin_access.web_request(rql='CWUser P WHERE P login "John"',
-                                   pageid='123', fname='view') as req:
+                                               pageid='123', fname='view',
+                                               session=req.session) as req:
                 ctrl = self.ctrl(req)
                 rset = self.john.as_rset()
                 rset.req = req
@@ -898,7 +899,8 @@
             self.assertEqual(deletes, [])
             inserts = get_pending_inserts(req)
             self.assertEqual(inserts, ['12:tags:13'])
-        with self.remote_calling('add_pending_inserts', [['12', 'tags', '14']]) as (_, req):
+        with self.remote_calling('add_pending_inserts', [['12', 'tags', '14']],
+                                 session=req.session) as (_, req):
             deletes = get_pending_deletes(req)
             self.assertEqual(deletes, [])
             inserts = get_pending_inserts(req)
@@ -917,7 +919,8 @@
             self.assertEqual(inserts, [])
             deletes = get_pending_deletes(req)
             self.assertEqual(deletes, ['12:tags:13'])
-        with self.remote_calling('add_pending_delete', ['12', 'tags', '14']) as (_, req):
+        with self.remote_calling('add_pending_delete', ['12', 'tags', '14'],
+                                 session=req.session) as (_, req):
             inserts = get_pending_inserts(req)
             self.assertEqual(inserts, [])
             deletes = get_pending_deletes(req)
@@ -931,9 +934,10 @@
             req.remove_pending_operations()
 
     def test_remove_pending_operations(self):
-        with self.remote_calling('add_pending_delete', ['12', 'tags', '13']):
+        with self.remote_calling('add_pending_delete', ['12', 'tags', '13']) as (_, req):
             pass
-        with self.remote_calling('add_pending_inserts', [['12', 'tags', '14']]) as (_, req):
+        with self.remote_calling('add_pending_inserts', [['12', 'tags', '14']],
+                                 session=req.session) as (_, req):
             inserts = get_pending_inserts(req)
             self.assertEqual(inserts, ['12:tags:14'])
             deletes = get_pending_deletes(req)
--- a/cubicweb/web/views/authentication.py	Fri Mar 10 18:00:13 2017 +0100
+++ b/cubicweb/web/views/authentication.py	Tue Mar 14 11:07:58 2017 +0100
@@ -20,10 +20,13 @@
 
 
 from logilab.common.deprecation import class_renamed
+from logilab.common.textutils import unormalize
 
 from cubicweb import AuthenticationError
+from cubicweb.utils import make_uid
 from cubicweb.view import Component
 from cubicweb.web import InvalidSession
+from cubicweb.server.session import Connection
 
 
 class NoAuthInfo(Exception): pass
@@ -98,6 +101,38 @@
     '("ie" instead of "ei")')
 
 
+class Session(object):
+    """In-memory user session
+    """
+
+    def __init__(self, repo, user):
+        self.user = user  # XXX deprecate and store only a login.
+        self.repo = repo
+        self.sessionid = make_uid(unormalize(user.login))
+        self.data = {}
+
+    def __unicode__(self):
+        return '<session %s (0x%x)>' % (unicode(self.user.login), id(self))
+
+    @property
+    def anonymous_session(self):
+        # XXX for now, anonymous_user only exists in webconfig (and testconfig).
+        # It will only be present inside all-in-one instance.
+        # there is plan to move it down to global config.
+        if not hasattr(self.repo.config, 'anonymous_user'):
+            # not a web or test config, no anonymous user
+            return False
+        return self.user.login == self.repo.config.anonymous_user()[0]
+
+    def new_cnx(self):
+        """Return a new Connection object linked to the session
+
+        The returned Connection will *not* be managed by the Session.
+        """
+        cnx = Connection(self.repo, self.user)
+        cnx.session = self
+        return cnx
+
 
 class RepositoryAuthenticationManager(object):
     """authenticate user associated to a request and check session validity"""
@@ -133,7 +168,7 @@
         # check session.login and not user.login, since in case of login by
         # email, login and cnx.login are the email while user.login is the
         # actual user login
-        if login and session.login != login:
+        if login and session.user.login != login:
             raise InvalidSession('login mismatch')
 
     def authenticate(self, req):
@@ -170,4 +205,6 @@
         raise AuthenticationError()
 
     def _authenticate(self, login, authinfo):
-        return self.repo.new_session(login, **authinfo)
+        with self.repo.internal_cnx() as cnx:
+            user = self.repo.authenticate_user(cnx, login, **authinfo)
+        return Session(self.repo, user)