[IMP] http improvement

- context manager request object (removes some ugly hacks)
- improve http error handling
- add lazyresponses

bzr revid: al@openerp.com-20131203190639-e8r1qm9wc82t8g4k

openerp/addons/base/ir/ir_http.py

@@ -3,6 +3,7 @@
 #----------------------------------------------------------
 import logging
 import re
+import sys
 
 import werkzeug.exceptions
 import werkzeug.routing
@@ -74,26 +75,29 @@ class ir_http(osv.AbstractModel):
         request.disable_db = True
         request.uid = None
 
-    def _authenticate(self, func, arguments):
+    def _authenticate(self, auth_method='user'):
-        auth_method = getattr(func, "auth", "user")
         if request.session.uid:
             try:
                 request.session.check_security()
                 # what if error in security.check()
                 #   -> res_users.check()
                 #   -> res_users.check_credentials()
-            except Exception:
+            except http.SessionExpiredException:
                 request.session.logout()
+                raise http.SessionExpiredException("Session expired for request %s" % request.httprequest)
         getattr(self, "_auth_method_%s" % auth_method)()
         return auth_method
 
-    def _handle_404(self, exception):
+    def _handle_exception(self, exception):
-        raise exception
+        if isinstance(exception, openerp.exceptions.AccessError):
+            code = 403
+        else:
+            code = getattr(exception, 'code', 500)
 
-    def _handle_403(self, exception):
+        fn = getattr(self, '_handle_%d' % code, self._handle_unknown_exception)
-        raise exception
+        return fn(exception)
 
-    def _handle_500(self, exception):
+    def _handle_unknown_exception(self, exception):
         raise exception
 
     def _dispatch(self):
@@ -101,13 +105,16 @@ class ir_http(osv.AbstractModel):
         try:
             func, arguments = self._find_handler()
         except werkzeug.exceptions.NotFound, e:
-            return self._handle_404(e)
+            return self._handle_exception(e)
 
         # check authentication level
         try:
-            auth_method = self._authenticate(func, arguments)
+            auth_method = self._authenticate(getattr(func, "auth", None))
-        except werkzeug.exceptions.NotFound, e:
+        except Exception:
-            return self._handle_403(e)
+            # force a Forbidden exception with the original traceback
+            return self._handle_exception(
+                convert_exception_to(
+                    werkzeug.exceptions.Forbidden))
 
         # post process arg to set uid on browse records
         for arg in arguments.itervalues():
@@ -121,9 +128,7 @@ class ir_http(osv.AbstractModel):
             if isinstance(result, Exception):
                 raise result
         except Exception, e:
-            fn = getattr(self, '_handle_%s' % getattr(e, 'code', 500),
+            return self._handle_exception(e)
-                         self._handle_500)
-            return fn(e)
 
         return result
 
@@ -139,4 +144,29 @@ class ir_http(osv.AbstractModel):
 
         return self._routing_map
 
+def convert_exception_to(to_type, with_message=False):
+    """ Should only be called from an exception handler. Fetches the current
+    exception data from sys.exc_info() and creates a new exception of type
+    ``to_type`` with the original traceback.
+
+    If ``with_message`` is ``True``, sets the new exception's message to be
+    the stringification of the original exception. If ``False``, does not
+    set the new exception's message. Otherwise, uses ``with_message`` as the
+    new exception's message.
+
+    :type with_message: str|bool
+    """
+    etype, original, tb = sys.exc_info()
+    try:
+        if with_message is False:
+            message = None
+        elif with_message is True:
+            message = str(original)
+        else:
+            message = str(with_message)
+
+        raise to_type, message, tb
+    except to_type, e:
+        return e
+
 # vim:et:

openerp/http.py

@@ -3,7 +3,6 @@
 # OpenERP HTTP layer
 #----------------------------------------------------------
 import ast
-import cgi
 import collections
 import contextlib
 import errno
@@ -36,13 +35,20 @@ import werkzeug.wsgi
 
 import openerp
 from openerp.service import security, model as service_model
-from openerp.tools import config
 
 _logger = logging.getLogger(__name__)
 
 #----------------------------------------------------------
 # RequestHandler
 #----------------------------------------------------------
+# Thread local global request object
+_request_stack = werkzeug.local.LocalStack()
+
+request = _request_stack()
+"""
+    A global proxy that always redirect to the current request object.
+"""
+
 class WebRequest(object):
     """ Parent class for all OpenERP Web request types, mostly deals with
     initialization and setup of the request object (the dispatching itself has
@@ -138,11 +144,24 @@ class WebRequest(object):
         trying to access this property will raise an exception.
         """
         # some magic to lazy create the cr
-        if not self._cr_cm:
+        if not self._cr:
-            self._cr_cm = self.registry.cursor()
+            self._cr = self.registry.db.cursor()
-            self._cr = self._cr_cm.__enter__()
         return self._cr
 
+    def __enter__(self):
+        _request_stack.push(self)
+        return self
+
+    def __exit__(self, exc_type, exc_value, traceback):
+        _request_stack.pop()
+        if self._cr:
+            if exc_type is None:
+                self._cr.commit()
+            self._cr.close()
+        # just to be sure no one tries to re-use the request
+        self.disable_db = True
+        self.uid = None
+
     def set_handler(self, func, arguments, auth):
         # is this needed ?
         arguments = dict((k, v) for k, v in arguments.iteritems()
@@ -154,39 +173,23 @@ class WebRequest(object):
         self.auth_method = auth
 
     def _call_function(self, *args, **kwargs):
-        try:
+        request = self
-            # ugly syntax only to get the __exit__ arguments to pass to self._cr
+        if self.func_request_type != self._request_type:
-            request = self
+            raise Exception("%s, %s: Function declared as capable of handling request of type '%s' but called with a request of type '%s'" \
-            class with_obj(object):
+                % (self.func, self.httprequest.path, self.func_request_type, self._request_type))
-                def __enter__(self):
-                    pass
-                def __exit__(self, *args):
-                    if request._cr_cm:
-                        request._cr_cm.__exit__(*args)
-                        request._cr_cm = None
-                        request._cr = None
 
-            with with_obj():
+        kwargs.update(self.func_arguments)
-                if self.func_request_type != self._request_type:
-                    raise Exception("%s, %s: Function declared as capable of handling request of type '%s' but called with a request of type '%s'" \
-                        % (self.func, self.httprequest.path, self.func_request_type, self._request_type))
 
-                kwargs.update(self.func_arguments)
+        # Backward for 7.0
-
+        if getattr(self.func, '_first_arg_is_req', False):
-                # Backward for 7.0
+            args = (request,) + args
-                if getattr(self.func, '_first_arg_is_req', False):
+        # Correct exception handling and concurency retry
-                    args = (request,) + args
+        @service_model.check
-                # Correct exception handling and concurency retry
+        def checked_call(dbname, *a, **kw):
-                @service_model.check
+            return self.func(*a, **kw)
-                def checked_call(dbname, *a, **kw):
+        if self.db:
-                    return self.func(*a, **kw)
+            return checked_call(self.db, *args, **kwargs)
-                if self.db:
+        return self.func(*args, **kwargs)
-                    return checked_call(self.db, *args, **kwargs)
-                return self.func(*args, **kwargs)
-        finally:
-            # just to be sure no one tries to re-use the request
-            self.disable_db = True
-            self.uid = None
 
     @property
     def debug(self):
@@ -197,7 +200,7 @@ class WebRequest(object):
         warnings.warn('please use request.registry and request.cr directly', DeprecationWarning)
         yield (self.registry, self.cr)
 
-def route(route, type="http", auth="user"):
+def route(route, type="http", auth="user", methods=None):
     """
     Decorator marking the decorated method as being a handler for requests. The method must be part of a subclass
     of ``Controller``.
@@ -214,6 +217,7 @@ def route(route, type="http", auth="user"):
         * ``none``: The method is always active, even if there is no database. Mainly used by the framework and
         authentication modules. There request code will not have any facilities to access the database nor have any
         configuration indicating the current database nor the current user.
+    :param methods: A sequence of http methods this route applies to. If not specified, all methods are allowed.
     """
     assert type in ["http", "json"]
     def decorator(f):
@@ -221,6 +225,7 @@ def route(route, type="http", auth="user"):
             f.routes = route
         else:
             f.routes = [route]
+        f.methods = methods
         f.exposed = type
         if getattr(f, "auth", None) is None:
             f.auth = auth
@@ -398,17 +403,8 @@ class HttpRequest(WebRequest):
     def dispatch(self):
         try:
             r = self._call_function(**self.params)
-        except werkzeug.exceptions.HTTPException, e:
+        except (werkzeug.exceptions.HTTPException), e:
             r = e
-        except Exception, e:
-            _logger.exception("An exception occured during an http request")
-            se = serialize_exception(e)
-            error = {
-                'code': 200,
-                'message': "OpenERP Server Error",
-                'data': se
-            }
-            r = werkzeug.exceptions.InternalServerError(cgi.escape(simplejson.dumps(error)))
         else:
             if not r:
                 r = werkzeug.wrappers.Response(status=204)  # no content
@@ -451,24 +447,6 @@ def httprequest(f):
         base = ""
     return route([base, base + "/<path:_ignored_path>"], type="http", auth="user")(f)
 
-#----------------------------------------------------------
-# Thread local global request object
-#----------------------------------------------------------
-_request_stack = werkzeug.local.LocalStack()
-
-request = _request_stack()
-"""
-    A global proxy that always redirect to the current request object.
-"""
-
-@contextlib.contextmanager
-def set_request(req):
-    _request_stack.push(req)
-    try:
-        yield
-    finally:
-        _request_stack.pop()
-
 #----------------------------------------------------------
 # Controller and route registration
 #----------------------------------------------------------
@@ -526,7 +504,7 @@ def routing_map(modules, nodb_only, converters=None):
                             url = o._cp_path.rstrip('/') + '/' + url.lstrip('/')
                             if url.endswith("/") and len(url) > 1:
                                 url = url[: -1]
-                        routing_map.add(werkzeug.routing.Rule(url, endpoint=mv))
+                        routing_map.add(werkzeug.routing.Rule(url, endpoint=mv, methods=mv.methods))
     return routing_map
 
 #----------------------------------------------------------
@@ -816,6 +794,18 @@ mimetypes.add_type('application/font-woff', '.woff')
 mimetypes.add_type('application/vnd.ms-fontobject', '.eot')
 mimetypes.add_type('application/x-font-ttf', '.ttf')
 
+class LazyResponse(werkzeug.wrappers.Response):
+    """ Lazy werkzeug response.
+    API not yet frozen"""
+
+    def __init__(self, callback, **kwargs):
+        super(LazyResponse, self).__init__(mimetype='text/html')
+        self.callback = callback
+        self.params = kwargs
+    def process(self):
+        response = self.callback(**self.params)
+        self.response.append(response)
+
 class DisableCacheMiddleware(object):
     def __init__(self, app):
         self.app = app
@@ -941,6 +931,12 @@ class Root(object):
             return HttpRequest(httprequest)
 
     def get_response(self, httprequest, result, explicit_session):
+        if isinstance(result, LazyResponse):
+            try:
+                result.process()
+            except(Exception), e:
+                result = request.registry['ir.http']._handle_exception(e)
+
         if isinstance(result, basestring):
             response = werkzeug.wrappers.Response(result, mimetype='text/html')
         else:
@@ -980,7 +976,7 @@ class Root(object):
                 result = request.dispatch()
                 return result
 
-            with set_request(request):
+            with request:
                 db = request.session.db
                 if db:
                     openerp.modules.registry.RegistryManager.check_registry_signaling(db)