Merge lp:~openerp-dev/openerp-web/7.0-controller-extension-xmo into lp:openerp-web/7.0

[Antony Lesuisse (OpenERP)]

[lp:~openerp-dev/openerp-web/7.0-controller-extension-xmo]

3880. By Xavier (Open ERP) on 2013-04-18

[FIX] Extension of controller in-place with explicit spec of same _cp_path

When extending a controller in-place (e.g. A(Controller), B(A)) and
providing the exact same _cp_path as parent (no-op) execution path
would go into handler for _cp_path overwriting and raise an assertion
error for overwriting of existing controller.

Except this is allowed (if ugly) pattern, so warn & ignore behavior
(it is harmless).

3881. By Xavier (Open ERP) on 2013-04-18

[IMP] only use name of class being created: module is incorrect for dynamically created classes

=== modified file 'addons/web/controllers/main.py'
--- addons/web/controllers/main.py	2013-04-05 13:47:47 +0000
+++ addons/web/controllers/main.py	2013-04-18 09:47:37 +0000
@@ -1,11 +1,9 @@
 # -*- coding: utf-8 -*-
-
 import ast
 import base64
 import csv
 import glob
 import itertools
-import logging
 import operator
 import datetime
 import hashlib
@@ -30,7 +28,6 @@
     xlwt = None
 
 import openerp
-import openerp.modules.registry
 from openerp.tools.translate import _
 
 from .. import http
@@ -1439,7 +1436,8 @@
         else:
             return False
 
-class Export(View):
+
+class Export(openerpweb.Controller):
     _cp_path = "/web/export"
 
     @openerpweb.jsonrequest
@@ -1580,7 +1578,13 @@
             (prefix + '/' + k, prefix_string + '/' + v)
             for k, v in self.fields_info(req, model, export_fields).iteritems())
 
-    #noinspection PyPropertyDefinition
+
+class ExportFormat(object):
+    """
+    Superclass for export formats, should probably be an abc and have a way to
+    generate _cp_path from fmt but it's a pain to deal with conflicts with
+    ControllerType
+    """
     @property
     def content_type(self):
         """ Provides the format's content type """
@@ -1621,14 +1625,14 @@
         else:
             columns_headers = [val['label'].strip() for val in fields]
 
-
         return req.make_response(self.from_data(columns_headers, import_data),
             headers=[('Content-Disposition',
                             content_disposition(self.filename(model), req)),
                      ('Content-Type', self.content_type)],
             cookies={'fileToken': int(token)})
 
-class CSVExport(Export):
+
+class CSVExport(ExportFormat, http.Controller):
     _cp_path = '/web/export/csv'
     fmt = {'tag': 'csv', 'label': 'CSV'}
 
@@ -1663,7 +1667,8 @@
         fp.close()
         return data
 
-class ExcelExport(Export):
+
+class ExcelExport(ExportFormat, http.Controller):
     _cp_path = '/web/export/xls'
     fmt = {
         'tag': 'xls',
@@ -1702,7 +1707,8 @@
         fp.close()
         return data
 
-class Reports(View):
+
+class Reports(openerpweb.Controller):
     _cp_path = "/web/report"
     POLLING_DELAY = 0.25
     TYPES_MAPPING = {

=== modified file 'addons/web/http.py'
--- addons/web/http.py	2013-03-01 17:16:16 +0000
+++ addons/web/http.py	2013-04-18 09:47:37 +0000
@@ -354,18 +354,64 @@
 #----------------------------------------------------------
 addons_module = {}
 addons_manifest = {}
-controllers_class = []
-controllers_object = {}
+controllers_class = {}
 controllers_path = {}
 
 class ControllerType(type):
     def __init__(cls, name, bases, attrs):
         super(ControllerType, cls).__init__(name, bases, attrs)
-        controllers_class.append(("%s.%s" % (cls.__module__, cls.__name__), cls))
+        # Only for root "Controller"
+        if bases == (object,):
+            assert name == 'Controller'
+            return
+
+        path = attrs.get('_cp_path')
+        if Controller in bases:
+            assert path, "Controller subclass %s missing a _cp_path" % name
+        else:
+            parent_paths = set(base._cp_path for base in bases
+                               if issubclass(base, Controller))
+            assert len(parent_paths) == 1,\
+                "%s inheriting from multiple controllers is not supported" % (
+                    name)
+            [parent_path] = parent_paths
+            [parent] = [
+                controller for controller in controllers_class.itervalues()
+                if controller._cp_path == parent_path]
+
+            # inherit from a Controller subclass
+            if path:
+                # if extending in place with same URL, ignore URL
+                if parent_path == path:
+                    _logger.warn(
+                        "Controller %s extending %s in-place should not "
+                        "explicitly specify URL", name, parent)
+                    return
+                _logger.warn("Re-exposing %s at %s.\n"
+                             "\tThis usage is unsupported.",
+                             parent.__name__,
+                             attrs['_cp_path'])
+
+        if path:
+            assert path not in controllers_class,\
+                "Trying to expose %s at the same URL as %s" % (
+                    name, controllers_class[path])
+            controllers_class[path] = cls
+
 
 class Controller(object):
     __metaclass__ = ControllerType
 
+    def __new__(cls, *args, **kwargs):
+        subclasses = [c for c in cls.__subclasses__()
+                      if c._cp_path == cls._cp_path]
+        if subclasses:
+            name = "%s (+%s)" % (
+                cls.__name__,
+                '+'.join(sub.__name__ for sub in subclasses))
+            cls = type(name, tuple(reversed(subclasses)), {})
+        return object.__new__(cls)
+
 #----------------------------------------------------------
 # Session context manager
 #----------------------------------------------------------
@@ -558,12 +604,8 @@
                         addons_manifest[module] = manifest
                         self.statics['/%s/static' % module] = path_static
 
-        for k, v in controllers_class:
-            if k not in controllers_object:
-                o = v()
-                controllers_object[k] = o
-                if hasattr(o, '_cp_path'):
-                    controllers_path[o._cp_path] = o
+        for c in controllers_class.itervalues():
+            controllers_path[c._cp_path] = c()
 
         app = werkzeug.wsgi.SharedDataMiddleware(self.dispatch, self.statics)
         self.dispatch = DisableCacheMiddleware(app)

=== modified file 'addons/web/tests/__init__.py'
--- addons/web/tests/__init__.py	2012-11-26 14:05:25 +0000
+++ addons/web/tests/__init__.py	2013-04-18 09:47:37 +0000
@@ -1,9 +1,10 @@
 # -*- coding: utf-8 -*-
-from . import test_dataset, test_menu, test_serving_base, test_js
+from . import test_dataset, test_menu, test_serving_base, test_js, test_dispatch
 
 fast_suite = []
 checks = [
     test_dataset,
     test_menu,
     test_serving_base,
+    test_dispatch,
 ]

=== added file 'addons/web/tests/test_dispatch.py'
--- addons/web/tests/test_dispatch.py	1970-01-01 00:00:00 +0000
+++ addons/web/tests/test_dispatch.py	2013-04-18 09:47:37 +0000
@@ -0,0 +1,373 @@
+# -*- coding: utf-8 -*-
+import contextlib
+import json
+import logging
+import logging.handlers
+import types
+import unittest2
+
+from .. import http
+import werkzeug.test
+
+
+def setUpModule():
+    """
+    Force load_addons once to import all the crap we don't care for as this
+    thing is full of side-effects
+    """
+    http.Root().load_addons()
+
+class DispatchCleanup(unittest2.TestCase):
+    """
+    Cleans up controllers registries in the web client so it's possible to
+    test controllers registration and dispatching in isolation.
+    """
+    def setUp(self):
+        self.classes = http.controllers_class
+        self.paths = http.controllers_path
+
+        http.controllers_class = {}
+        http.controllers_path = {}
+
+    def tearDown(self):
+        http.controllers_path = self.paths
+        http.controllers_class = self.classes
+
+
+def jsonrpc_body(params=None):
+    """
+    Builds and dumps the body of a JSONRPC request with params ``params``
+    """
+    return json.dumps({
+        'jsonrpc': '2.0',
+        'method': 'call',
+        'id': None,
+        'params': params or {},
+    })
+
+
+def jsonrpc_response(result=None):
+    """
+    Builds a JSONRPC response (as a Python dict) with result ``result``
+    """
+    return {
+        u'jsonrpc': u'2.0',
+        u'id': None,
+        u'result': result,
+    }
+
+
+class TestHandler(logging.handlers.BufferingHandler):
+    def __init__(self):
+        logging.handlers.BufferingHandler.__init__(self, 0)
+
+    def shouldFlush(self, record):
+        return False
+
[e-mail]
+def capture_logging(logger, level=logging.DEBUG):
+    logger = logging.getLogger(logger)
+    old_level = logger.level
+    old_handlers = logger.handlers
+    old_propagate = logger.propagate
+
+    test_handler = TestHandler()
+    logger.handlers = [test_handler]
+    logger.setLevel(level)
+    logger.propagate = False
+
+    try:
+        yield test_handler
+    finally:
+        logger.propagate = old_propagate
+        logger.setLevel(old_level)
+        logger.handlers = old_handlers
+
+
+class TestDispatching(DispatchCleanup):
+    def setUp(self):
+        super(TestDispatching, self).setUp()
+        self.app = http.Root()
+        self.client = werkzeug.test.Client(self.app)
+
+    def test_not_exposed(self):
+        class CatController(http.Controller):
+            _cp_path = '/cat'
+
+            def index(self):
+                return 'Blessid iz da feline'
+
+        self.app.load_addons()
+
+        body, status, headers = self.client.get('/cat')
+        self.assertEqual('404 NOT FOUND', status)
+
+    def test_basic_http(self):
+        class CatController(http.Controller):
+            _cp_path = '/cat'
+
+            @http.httprequest
+            def index(self, req):
+                return 'no walk in counsil of wickid,'
+
+        self.app.load_addons()
+
+        body, status, headers = self.client.get('/cat')
+        self.assertEqual('200 OK', status)
+        self.assertEqual('no walk in counsil of wickid,', ''.join(body))
+
+    def test_basic_jsonrpc(self):
+        class CatController(http.Controller):
+            _cp_path = '/cat'
+
+            @http.jsonrequest
+            def index(self, req):
+                return 'no place paws in path of da sinnerz,'
+        self.app.load_addons()
+
+        body, status, headers = self.client.post('/cat', data=jsonrpc_body())
+
+        self.assertEqual('200 OK', status)
+        self.assertEqual(
+            jsonrpc_response('no place paws in path of da sinnerz,'),
+            json.loads(''.join(body)))
+
+
+class TestSubclassing(DispatchCleanup):
+    def setUp(self):
+        super(TestSubclassing, self).setUp()
+        self.app = http.Root()
+        self.client = werkzeug.test.Client(self.app)
+
+    def test_add_method(self):
+        class CatController(http.Controller):
+            _cp_path = '/cat'
+
+            @http.httprequest
+            def index(self, req):
+                return 'no sit and purr with da mockerz.'
+
+        class CeilingController(CatController):
+            @http.httprequest
+            def lol(self, req):
+                return 'But der delightz in lawz of Ceiling Cat,'
+
+        self.app.load_addons()
+
+        body, status, headers = self.client.get('/cat')
+        self.assertEqual('200 OK', status)
+        self.assertEqual('no sit and purr with da mockerz.', ''.join(body))
+        body, status, headers = self.client.get('/cat/lol')
+        self.assertEqual('200 OK', status)
+        self.assertEqual('But der delightz in lawz of Ceiling Cat,',
+                         ''.join(body))
+
+    def test_override_method(self):
+        class CatController(http.Controller):
+            _cp_path = '/cat'
+
+            @http.httprequest
+            def index(self, req):
+                return 'an ponderz'
+
+        class CeilingController(CatController):
+            @http.httprequest
+            def index(self, req):
+                return '%s much.' % super(CeilingController, self).index(req)
+
+        self.app.load_addons()
+
+        body, status, headers = self.client.get('/cat')
+        self.assertEqual('200 OK', status)
+        self.assertEqual('an ponderz much.', ''.join(body))
+
+    def test_make_invisible(self):
+        class CatController(http.Controller):
+            _cp_path = '/cat'
+
+            @http.httprequest
+            def index(self, req):
+                return 'Tehy liek treez bai teh waterz,'
+
+        class CeilingController(CatController):
+            def index(self, req):
+                return super(CeilingController, self).index(req)
+
+        self.app.load_addons()
+
+        body, status, headers = self.client.get('/cat')
+        self.assertEqual('404 NOT FOUND', status)
+
+    def test_make_json_invisible(self):
+        class CatController(http.Controller):
+            _cp_path = '/cat'
+
+            @http.jsonrequest
+            def index(self, req):
+                return 'Tehy liek treez bai teh waterz,'
+
+        class CeilingController(CatController):
+            def index(self, req):
+                return super(CeilingController, self).index(req)
+
+        self.app.load_addons()
+
+        body, status, headers = self.client.post('/cat')
+        self.assertEqual('404 NOT FOUND', status)
+
+    def test_extends(self):
+        """
+        When subclassing an existing Controller new classes are "merged" into
+        the base one
+        """
+        class A(http.Controller):
+            _cp_path = '/foo'
+            @http.httprequest
+            def index(self, req):
+                return '1'
+
+        class B(A):
+            @http.httprequest
+            def index(self, req):
+                return "%s 2" % super(B, self).index(req)
+
+        class C(A):
+            @http.httprequest
+            def index(self, req):
+                return "%s 3" % super(C, self).index(req)
+
+        self.app.load_addons()
+
+        body, status, headers = self.client.get('/foo')
+        self.assertEqual('200 OK', status)
+        self.assertEqual('1 2 3', ''.join(body))
+
+    def test_extends_same_path(self):
+        """
+        When subclassing an existing Controller and specifying the same
+        _cp_path as the parent, ???
+        """
+        class A(http.Controller):
+            _cp_path = '/foo'
+            @http.httprequest
+            def index(self, req):
+                return '1'
+
+        class B(A):
+            _cp_path = '/foo'
+            @http.httprequest
+            def index(self, req):
+                return '2'
+
+        self.app.load_addons()
+
+        body, status, headers = self.client.get('/foo')
+        self.assertEqual('200 OK', status)
+        self.assertEqual('2', ''.join(body))
+
+    def test_re_expose(self):
+        """
+        An existing Controller should not be extended with a new cp_path
+        (re-exposing somewhere else)
+        """
+        class CatController(http.Controller):
+            _cp_path = '/cat'
+
+            @http.httprequest
+            def index(self, req):
+                return '[%s]' % self.speak()
+
+            def speak(self):
+                return 'Yu ordered cheezburgerz,'
+
+        with capture_logging('openerp.addons.web.http') as handler:
+            class DogController(CatController):
+                _cp_path = '/dog'
+
+                def speak(self):
+                    return 'Woof woof woof woof'
+
+            [record] = handler.buffer
+            self.assertEqual(logging.WARN, record.levelno)
+            self.assertEqual("Re-exposing CatController at /dog.\n"
+                             "\tThis usage is unsupported.",
+                             record.getMessage())
+
+    def test_fail_redefine(self):
+        """
+        An existing Controller can't be overwritten by a new one on the same
+        path (? or should this generate a warning and still work as if it was
+        an extend?)
+        """
+        class FooController(http.Controller):
+            _cp_path = '/foo'
+
+        with self.assertRaises(AssertionError):
+            class BarController(http.Controller):
+                _cp_path = '/foo'
+
+    def test_fail_no_path(self):
+        """
+        A Controller must have a path (and thus be exposed)
+        """
+        with self.assertRaises(AssertionError):
+            class FooController(http.Controller):
+                pass
+
+    def test_mixin(self):
+        """
+        Can mix "normal" python classes into a controller directly
+        """
+        class Mixin(object):
+            @http.httprequest
+            def index(self, req):
+                return 'ok'
+
+        class FooController(http.Controller, Mixin):
+            _cp_path = '/foo'
+
+        class BarContoller(Mixin, http.Controller):
+            _cp_path = '/bar'
+
+        self.app.load_addons()
+
+        body, status, headers = self.client.get('/foo')
+        self.assertEqual('200 OK', status)
+        self.assertEqual('ok', ''.join(body))
+
+        body, status, headers = self.client.get('/bar')
+        self.assertEqual('200 OK', status)
+        self.assertEqual('ok', ''.join(body))
+
+    def test_mixin_extend(self):
+        """
+        Can mix "normal" python class into a controller by extension
+        """
+        class FooController(http.Controller):
+            _cp_path = '/foo'
+
+        class M1(object):
+            @http.httprequest
+            def m1(self, req):
+                return 'ok 1'
+
+        class M2(object):
+            @http.httprequest
+            def m2(self, req):
+                return 'ok 2'
+
+        class AddM1(FooController, M1):
+            pass
+
+        class AddM2(M2, FooController):
+            pass
+
+        self.app.load_addons()
+
+        body, status, headers = self.client.get('/foo/m1')
+        self.assertEqual('200 OK', status)
+        self.assertEqual('ok 1', ''.join(body))
+
+        body, status, headers = self.client.get('/foo/m2')
+        self.assertEqual('200 OK', status)
+        self.assertEqual('ok 2', ''.join(body))

=== modified file 'addons/web_diagram/controllers/main.py'
--- addons/web_diagram/controllers/main.py	2012-12-06 09:51:23 +0000
+++ addons/web_diagram/controllers/main.py	2013-04-18 09:47:37 +0000
@@ -1,9 +1,10 @@
-import openerp
-
-class DiagramView(openerp.addons.web.controllers.main.View):
+from openerp.addons.web.http import Controller, jsonrequest
+
+
+class Diagram(Controller):
     _cp_path = "/web_diagram/diagram"
 
-    @openerp.addons.web.http.jsonrequest
+    @jsonrequest
     def get_diagram_info(self, req, id, model, node, connector,
                          src_node, des_node, label, **kw):