[web/application] remove `path` argument from CubicwebPublisher methods
authorLaura Médioni <laura.medioni@logilab.fr>
Fri, 21 Oct 2016 13:09:47 +0200
changeset 11725 904ee9cd0cf9
parent 11724 0fe3cf5c06b3
child 11726 a599e23c5712
[web/application] remove `path` argument from CubicwebPublisher methods Path can actually be accessed from `req` object. This allows to avoid duplicating this information. This cset prepares the next ones that aim at adding the language as a prefix of the relative path.
cubicweb/devtools/fake.py
cubicweb/devtools/testlib.py
cubicweb/etwist/server.py
cubicweb/web/application.py
cubicweb/web/test/unittest_application.py
cubicweb/web/test/unittest_views_basecontrollers.py
cubicweb/web/test/unittest_views_staticcontrollers.py
cubicweb/wsgi/handler.py
--- a/cubicweb/devtools/fake.py	Thu Oct 20 18:28:46 2016 +0200
+++ b/cubicweb/devtools/fake.py	Fri Oct 21 13:09:47 2016 +0200
@@ -65,7 +65,9 @@
             kwargs['vreg'] = CWRegistryStore(FakeConfig(), initlog=False)
         kwargs['https'] = False
         self._http_method = kwargs.pop('method', 'GET')
-        self._url = kwargs.pop('url', None) or 'view?rql=Blop&vid=blop'
+        self._url = kwargs.pop('url', None)
+        if self._url is None:
+            self._url = 'view?rql=Blop&vid=blop'
         super(FakeRequest, self).__init__(*args, **kwargs)
         self._session_data = {}
 
--- a/cubicweb/devtools/testlib.py	Thu Oct 20 18:28:46 2016 +0200
+++ b/cubicweb/devtools/testlib.py	Fri Oct 21 13:09:47 2016 +0200
@@ -717,8 +717,9 @@
             ctrl = self.vreg['controllers'].select('ajax', req)
             yield ctrl.publish(), req
 
-    def app_handle_request(self, req, path='view'):
-        return self.app.core_handle(req, path)
+    @application._deprecated_path_arg
+    def app_handle_request(self, req):
+        return self.app.core_handle(req)
 
     @deprecated("[3.15] app_handle_request is the new and better way"
                 " (beware of small semantic changes)")
@@ -859,7 +860,9 @@
         """call the publish method of the application publisher, expecting to
         get a Redirect exception
         """
-        self.app_handle_request(req, path)
+        if req.relative_path(False) != path:
+            req._url = path
+        self.app_handle_request(req)
         self.assertTrue(300 <= req.status_out < 400, req.status_out)
         location = req.get_response_header('location')
         return self._parse_location(req, location)
--- a/cubicweb/etwist/server.py	Thu Oct 20 18:28:46 2016 +0200
+++ b/cubicweb/etwist/server.py	Fri Oct 21 13:09:47 2016 +0200
@@ -134,12 +134,10 @@
             # XXX should occur before authentication?
             path = self.url_rewriter.rewrite(host, origpath, request)
             request.uri.replace(origpath, path, 1)
-        else:
-            path = origpath
         req = CubicWebTwistedRequestAdapter(request, self.appli.vreg, https)
         try:
             ### Try to generate the actual request content
-            content = self.appli.handle_request(req, path)
+            content = self.appli.handle_request(req)
         except DirectResponse as ex:
             return ex.response
         # at last: create twisted object
--- a/cubicweb/web/application.py	Thu Oct 20 18:28:46 2016 +0200
+++ b/cubicweb/web/application.py	Fri Oct 21 13:09:47 2016 +0200
@@ -20,13 +20,14 @@
 __docformat__ = "restructuredtext en"
 
 import contextlib
+from functools import wraps
 import json
 import sys
 from time import clock, time
 from contextlib import contextmanager
 from warnings import warn
 
-from six import text_type, binary_type
+from six import PY2, text_type, binary_type
 from six.moves import http_client
 
 from rql import BadRQLQuery
@@ -47,6 +48,30 @@
 SESSION_MANAGER = None
 
 
+def _deprecated_path_arg(func):
+    @wraps(func)
+    def wrapper(self, req, *args, **kwargs):
+        if args or 'path' in kwargs:
+            func_name = func.func_name if PY2 else func.__name__
+            warn('[3.24] path argument got removed from "%s" parameters' % func_name,
+                 DeprecationWarning)
+        return func(self, req)
+    return wrapper
+
+
+def _deprecated_req_path_swapped(func):
+    @wraps(func)
+    def wrapper(self, req, *args, **kwargs):
+        if not isinstance(req, CubicWebRequestBase):
+            warn('[3.15] Application entry point arguments are now (req, path) '
+                 'not (path, req)', DeprecationWarning, 2)
+            path = req
+            req = args[0] if args else kwargs.pop('req')
+            args = (path, ) + args[1:]
+        return func(self, req, *args, **kwargs)
+    return wrapper
+
+
 @contextmanager
 def anonymized_request(req):
     orig_cnx = req.cnx
@@ -198,7 +223,8 @@
 
     # publish methods #########################################################
 
-    def log_handle_request(self, req, path):
+    @_deprecated_path_arg
+    def log_handle_request(self, req):
         """wrapper around _publish to log all queries executed for a given
         accessed path
         """
@@ -224,7 +250,7 @@
 
         req.set_cnx = wrap_set_cnx(req.set_cnx)
         try:
-            return self.main_handle_request(req, path)
+            return self.main_handle_request(req)
         finally:
             cnx = req.cnx
             if cnx:
@@ -240,20 +266,17 @@
                     except Exception:
                         self.exception('error while logging queries')
 
-    def main_handle_request(self, req, path):
-        """Process an http request
+    @_deprecated_req_path_swapped
+    @_deprecated_path_arg
+    def main_handle_request(self, req):
+        """Process an HTTP request `req`
 
-        Arguments are:
-        - a Request object
-        - path of the request object
+        :type req: `web.Request`
+        :param req: the request object
 
         It returns the content of the http response. HTTP header and status are
         set on the Request object.
         """
-        if not isinstance(req, CubicWebRequestBase):
-            warn('[3.15] Application entry point arguments are now (req, path) '
-                 'not (path, req)', DeprecationWarning, 2)
-            req, path = path, req
         if req.authmode == 'http':
             # activate realm-based auth
             realm = self.vreg.config['realm']
@@ -280,7 +303,7 @@
             try:
                 # Try to generate the actual request content
                 with cnx:
-                    content = self.core_handle(req, path)
+                    content = self.core_handle(req)
             # Handle user log-out
             except LogOut as ex:
                 # When authentification is handled by cookie the code that
@@ -329,24 +352,23 @@
         assert isinstance(content, binary_type)
         return content
 
-    def core_handle(self, req, path):
-        """method called by the main publisher to process <path>
+    @_deprecated_path_arg
+    def core_handle(self, req):
+        """method called by the main publisher to process <req> relative path
 
         should return a string containing the resulting page or raise a
         `NotFound` exception
 
-        :type path: str
-        :param path: the path part of the url to publish
-
         :type req: `web.Request`
         :param req: the request object
 
         :rtype: str
         :return: the result of the pusblished url
         """
+        path = req.relative_path(False)
         # don't log form values they may contains sensitive information
-        self.debug('publish "%s" (%s, form params: %s)',
-                   path, req.session.sessionid, list(req.form))
+        self.debug('publish "%s" (%s, form params: %s)', path,
+                   req.session.sessionid, list(req.form))
         # remove user callbacks on a new request (except for json controllers
         # to avoid callbacks being unregistered before they could be called)
         tstart = clock()
@@ -423,7 +445,7 @@
                 except Exception:
                     pass  # ignore rollback error at this point
         self.add_undo_link_to_msg(req)
-        self.debug('query %s executed in %s sec', req.relative_path(), clock() - tstart)
+        self.debug('query %s executed in %s sec', path, clock() - tstart)
         return result
 
     # Error handlers
--- a/cubicweb/web/test/unittest_application.py	Thu Oct 20 18:28:46 2016 +0200
+++ b/cubicweb/web/test/unittest_application.py	Fri Oct 21 13:09:47 2016 +0200
@@ -510,15 +510,15 @@
 
         with self.temporary_appobjects(ErrorAjaxView):
             with real_error_handling(self.app) as app:
-                with self.admin_access.web_request(vid='test.ajax.error') as req:
+                with self.admin_access.web_request(vid='test.ajax.error', url='') as req:
                     req.ajax_request = True
-                    app.handle_request(req, '')
+                    app.handle_request(req)
         self.assertEqual(http_client.INTERNAL_SERVER_ERROR,
                          req.status_out)
 
     def _test_cleaned(self, kwargs, injected, cleaned):
         with self.admin_access.web_request(**kwargs) as req:
-            page = self.app_handle_request(req, 'view')
+            page = self.app_handle_request(req)
             self.assertNotIn(injected.encode('ascii'), page)
             self.assertIn(cleaned.encode('ascii'), page)
 
@@ -556,20 +556,21 @@
     def test_http_auth_no_anon(self):
         req, origsession = self.init_authentication('http')
         self.assertAuthFailure(req)
-        self.app.handle_request(req, 'login')
+        self.app.handle_request(req)
         self.assertEqual(401, req.status_out)
         clear_cache(req, 'get_authorization')
         authstr = base64.encodestring(('%s:%s' % (self.admlogin, self.admpassword)).encode('ascii'))
         req.set_request_header('Authorization', 'basic %s' % authstr.decode('ascii'))
         self.assertAuthSuccess(req, origsession)
-        self.assertRaises(LogOut, self.app_handle_request, req, 'logout')
+        req._url = 'logout'
+        self.assertRaises(LogOut, self.app_handle_request, req)
         self.assertEqual(len(self.open_sessions), 0)
 
     def test_cookie_auth_no_anon(self):
         req, origsession = self.init_authentication('cookie')
         self.assertAuthFailure(req)
         try:
-            form = self.app.handle_request(req, 'login')
+            form = self.app.handle_request(req)
         except Redirect:
             self.fail('anonymous user should get login form')
         clear_cache(req, 'get_authorization')
@@ -579,7 +580,8 @@
         req.form['__login'] = self.admlogin
         req.form['__password'] = self.admpassword
         self.assertAuthSuccess(req, origsession)
-        self.assertRaises(LogOut, self.app_handle_request, req, 'logout')
+        req._url = 'logout'
+        self.assertRaises(LogOut, self.app_handle_request, req)
         self.assertEqual(len(self.open_sessions), 0)
 
     def test_login_by_email(self):
@@ -594,7 +596,8 @@
         req.form['__login'] = address
         req.form['__password'] = self.admpassword
         self.assertAuthSuccess(req, origsession)
-        self.assertRaises(LogOut, self.app_handle_request, req, 'logout')
+        req._url = 'logout'
+        self.assertRaises(LogOut, self.app_handle_request, req)
         self.assertEqual(len(self.open_sessions), 0)
 
     def _reset_cookie(self, req):
@@ -639,7 +642,8 @@
         authstr = base64.encodestring(('%s:%s' % (self.admlogin, self.admpassword)).encode('ascii'))
         req.set_request_header('Authorization', 'basic %s' % authstr.decode('ascii'))
         self.assertAuthSuccess(req, origsession)
-        self.assertRaises(LogOut, self.app_handle_request, req, 'logout')
+        req._url = 'logout'
+        self.assertRaises(LogOut, self.app_handle_request, req)
         self.assertEqual(len(self.open_sessions), 0)
 
     def test_cookie_auth_anon_allowed(self):
@@ -651,7 +655,8 @@
         req.form['__login'] = self.admlogin
         req.form['__password'] = self.admpassword
         self.assertAuthSuccess(req, origsession)
-        self.assertRaises(LogOut, self.app_handle_request, req, 'logout')
+        req._url = 'logout'
+        self.assertRaises(LogOut, self.app_handle_request, req)
         self.assertEqual(0, len(self.open_sessions))
 
     def test_anonymized_request(self):
@@ -672,6 +677,18 @@
             req.form['rql'] = 'rql:Any OV1, X WHERE X custom_workflow OV1?'
             self.app_handle_request(req)
 
+    def test_handle_deprecation(self):
+        """Test deprecation warning for *_handle methods."""
+        with self.admin_access.web_request(url='nothing') as req:
+            with self.assertWarns(DeprecationWarning) as cm:
+                self.app.core_handle(req, 'foo')
+            self.assertIn('path argument got removed from "core_handle"',
+                          str(cm.warning))
+            with self.assertWarns(DeprecationWarning) as cm:
+                self.app.main_handle_request('foo', req)
+            self.assertIn('entry point arguments are now (req, path)',
+                          str(cm.warning))
+
 
 if __name__ == '__main__':
     unittest_main()
--- a/cubicweb/web/test/unittest_views_basecontrollers.py	Thu Oct 20 18:28:46 2016 +0200
+++ b/cubicweb/web/test/unittest_views_basecontrollers.py	Fri Oct 21 13:09:47 2016 +0200
@@ -708,7 +708,7 @@
 
 
     def test_nonregr_rollback_on_validation_error(self):
-        with self.admin_access.web_request() as req:
+        with self.admin_access.web_request(url='edit') as req:
             p = self.create_user(req, u"doe")
             # do not try to skip 'primary_email' for this test
             old_skips = p.__class__.skip_copy_for
@@ -729,7 +729,7 @@
                 #    which fires a Redirect
                 # 2/ When re-publishing the copy form, the publisher implicitly commits
                 try:
-                    self.app_handle_request(req, 'edit')
+                    self.app_handle_request(req)
                 except Redirect:
                     req.form['rql'] = 'Any X WHERE X eid %s' % p.eid
                     req.form['vid'] = 'copy'
--- a/cubicweb/web/test/unittest_views_staticcontrollers.py	Thu Oct 20 18:28:46 2016 +0200
+++ b/cubicweb/web/test/unittest_views_staticcontrollers.py	Fri Oct 21 13:09:47 2016 +0200
@@ -35,7 +35,7 @@
     def _publish_static_files(self, url, header={}):
         with self.admin_access.web_request(headers=header) as req:
             req._url = url
-            self.app_handle_request(req, url)
+            self.app_handle_request(req)
             yield req
 
 class StaticControllerCacheTC(staticfilespublishermixin, CubicWebTC):
@@ -127,7 +127,7 @@
             url = head.concat_urls([req.data_url(js_file)
                                     for js_file in js_files])[len(req.base_url()):]
             req._url = url
-            res = self.app_handle_request(req, url)
+            res = self.app_handle_request(req)
             yield res, req
 
     def expected_content(self, js_files):
--- a/cubicweb/wsgi/handler.py	Thu Oct 20 18:28:46 2016 +0200
+++ b/cubicweb/wsgi/handler.py	Fri Oct 21 13:09:47 2016 +0200
@@ -108,8 +108,7 @@
         """this function performs the actual rendering
         """
         try:
-            path = req.path
-            result = self.appli.handle_request(req, path)
+            result = self.appli.handle_request(req)
         except DirectResponse as ex:
             return ex.response
         return WSGIResponse(req.status_out, req, result)