[IMP] base, web: static xml templates support serverside inheritance

QWeb templates that show up in the 'qweb' key of a module's manifest
now support server side inheritance and xpath evaluation

QWeb templates that show up in the xmlDependencies of a JS widget are not
impacted at all by theses changes, as they are served through the
Werkzeug sharedMiddleware

A similar syntax than ir.ui.view has been implemented in the QWeb templates
- each template must have a root node, whatever tag works

- the root node of a template must have a t-name containing the name of the template
The name -- without the module's name -- may contain dots pretty much anywhere
Though what is recommended is only underscores in template names

- if a template is to inherit from a parent, the root node has a t-inherit directive
containing either the full name of the template it inherits from which is module_name.template_name
or the name of the template, no module name necessary, if the parent template is in the same module

- there are 2 modes of inheriting
primary: copy the behavior of the parent into the template
extension: modifies the parent in place

Task: 1999528

closes odoo/odoo#33892

Signed-off-by: VincentSchippefilt <VincentSchippefilt@users.noreply.github.com>


Co-authored-by: Julien Mougenot <jum@odoo.com>
Co-authored-by: Lucas Perais <lpe@odoo.com>

addons/web/controllers/main.py

@@ -3,6 +3,7 @@
 
 import babel.messages.pofile
 import base64
+import copy
 import datetime
 import functools
 import glob
@@ -19,16 +20,15 @@ import re
 import sys
 import tempfile
 import time
-import zlib
 
 import werkzeug
 import werkzeug.exceptions
 import werkzeug.utils
 import werkzeug.wrappers
 import werkzeug.wsgi
-from collections import OrderedDict
+from collections import OrderedDict, defaultdict
 from werkzeug.urls import url_decode, iri_to_uri
-from xml.etree import ElementTree
+from lxml import etree
 import unicodedata
 
 
@@ -36,7 +36,7 @@ import odoo
 import odoo.modules.registry
 from odoo.api import call_kw, Environment
 from odoo.modules import get_module_path, get_resource_path
-from odoo.tools import image_process, topological_sort, html_escape, pycompat, ustr
+from odoo.tools import image_process, topological_sort, html_escape, pycompat, ustr, apply_inheritance_specs
 from odoo.tools.mimetypes import guess_mimetype
 from odoo.tools.translate import _
 from odoo.tools.misc import str2bool, xlwt, file_open
@@ -67,6 +67,8 @@ CONTENT_MAXAGE = 60 * 60 * 24 * 356
 
 DBNAME_PATTERN = '^[a-zA-Z0-9][a-zA-Z0-9_.-]+$'
 
+COMMENT_PATTERN = r'Modified by [\s\w\-.]+ from [\s\w\-.]+'
+
 #----------------------------------------------------------
 # Odoo Web helpers
 #----------------------------------------------------------
@@ -195,37 +197,6 @@ def module_boot(db=None):
     addons = serverside + dbside
     return addons
 
-def concat_xml(file_list):
-    """Concatenate xml files
-
-    :param list(str) file_list: list of files to check
-    :returns: (concatenation_result, checksum)
-    :rtype: (str, str)
-    """
-    checksum = hashlib.new('sha1')
-    if not file_list:
-        return '', checksum.hexdigest()
-
-    root = None
-    for fname in file_list:
-        with open(fname, 'rb') as fp:
-            contents = fp.read()
-            checksum.update(contents)
-            fp.seek(0)
-            try:
-                xml = ElementTree.parse(fp).getroot()
-            except ElementTree.ParseError as e:
-                _logger.error("Could not parse file %s: %s" % (fname, e.msg))
-                raise e
-
-        if root is None:
-            root = ElementTree.Element(xml.tag)
-        #elif root.tag != xml.tag:
-        #    raise ValueError("Root tags missmatch: %r != %r" % (root.tag, xml.tag))
-
-        for child in xml.getchildren():
-            root.append(child)
-    return ElementTree.tostring(root, 'utf-8'), checksum.hexdigest()
 
 def fs2web(path):
     """convert FS path into web path"""
@@ -234,8 +205,7 @@ def fs2web(path):
 def manifest_glob(extension, addons=None, db=None, include_remotes=False):
     if addons is None:
         addons = module_boot(db=db)
-    else:
-        addons = addons.split(',')
+
     r = []
     for addon in addons:
         manifest = http.addons_manifest.get(addon, None)
@@ -247,12 +217,13 @@ def manifest_glob(extension, addons=None, db=None, include_remotes=False):
         for pattern in globlist:
             if pattern.startswith(('http://', 'https://', '//')):
                 if include_remotes:
-                    r.append((None, pattern))
+                    r.append((None, pattern, addon))
             else:
                 for path in glob.glob(os.path.normpath(os.path.join(addons_path, addon, pattern))):
-                    r.append((path, fs2web(path[len(addons_path):])))
+                    r.append((path, fs2web(path[len(addons_path):]), addon))
     return r
 
+
 def manifest_list(extension, mods=None, db=None, debug=None):
     """ list resources to load specifying either:
     mods: a comma separated string listing modules
@@ -260,8 +231,9 @@ def manifest_list(extension, mods=None, db=None, debug=None):
     """
     if debug is not None:
         _logger.warning("odoo.addons.web.main.manifest_list(): debug parameter is deprecated")
+    mods = mods.split(',')
     files = manifest_glob(extension, addons=mods, db=db, include_remotes=True)
-    return [wp for _fp, wp in files]
+    return [wp for _fp, wp, addon in files]
 
 def get_last_modified(files):
     """ Returns the modification time of the most recently modified
@@ -450,6 +422,191 @@ def _admin_password_warn(uid):
             subtype='mail.mt_comment'
         )
 
+
+class HomeStaticTemplateHelpers(object):
+    """
+    Helper Class that wraps the reading of static qweb templates files
+    and xpath inheritance applied to those templates
+    /!\ Template inheritance order is defined by ir.module.module natural order
+        which is "sequence, name"
+        Then a topological sort is applied, which just puts dependencies
+        of a module before that module
+    """
+    NAME_TEMPLATE_DIRECTIVE = 't-name'
+    STATIC_INHERIT_DIRECTIVE = 't-inherit'
+    STATIC_INHERIT_MODE_DIRECTIVE = 't-inherit-mode'
+    PRIMARY_MODE = 'primary'
+    EXTENSION_MODE = 'extension'
+    DEFAULT_MODE = PRIMARY_MODE
+
+    def __init__(self, addons, db, checksum_only=False, debug=False):
+        '''
+        :param str|list addons: plain list or comma separated list of addons
+        :param str db: the current db we are working on
+        :param bool checksum_only: only computes the checksum of all files for addons
+        :param str debug: the debug mode of the session
+        '''
+        super(HomeStaticTemplateHelpers, self).__init__()
+        self.addons = addons.split(',') if isinstance(addons, str) else addons
+        self.db = db
+        self.debug = debug
+        self.checksum_only = checksum_only
+        self.template_dict = OrderedDict()
+
+    def _get_parent_template(self, addon, template):
+        """Computes the real addon name and the template name
+        of the parent template (the one that is inherited from)
+
+        :param str addon: the addon the template is declared in
+        :param etree template: the current template we are are handling
+        :returns: (str, str)
+        """
+        original_template_name = template.attrib[self.STATIC_INHERIT_DIRECTIVE]
+        split_name_attempt = original_template_name.split('.', 1)
+        parent_addon, parent_name = tuple(split_name_attempt) if len(split_name_attempt) == 2 else (addon, original_template_name)
+        if parent_addon not in self.template_dict:
+            if original_template_name in self.template_dict[addon]:
+                parent_addon = addon
+                parent_name = original_template_name
+            else:
+                raise ValueError(_('Module %s not loaded or inexistent, or templates of addon being loaded (%s) are misordered') % (parent_addon, addon))
+
+        if parent_name not in self.template_dict[parent_addon]:
+            raise ValueError(_("No template found to inherit from. Module %s and template name %s") % (parent_addon, parent_name))
+
+        return parent_addon, parent_name
+
+    def _compute_xml_tree(self, addon, file_name, source):
+        """Computes the xml tree that 'source' contains
+        Applies inheritance specs in the process
+
+        :param str addon: the current addon we are reading files for
+        :param str file_name: the current name of the file we are reading
+        :param str source: the content of the file
+        :returns: etree
+        """
+        try:
+            all_templates_tree = etree.parse(io.BytesIO(source), parser=etree.XMLParser(remove_comments=True)).getroot()
+        except etree.ParseError as e:
+            _logger.error("Could not parse file %s: %s" % (file_name, e.msg))
+            raise e
+
+        self.template_dict.setdefault(addon, OrderedDict())
+        for template_tree in list(all_templates_tree):
+            if self.NAME_TEMPLATE_DIRECTIVE in template_tree.attrib:
+                template_name = template_tree.attrib[self.NAME_TEMPLATE_DIRECTIVE]
+            else:
+                # self.template_dict[addon] grows after processing each template
+                template_name = 'anonymous_template_%s' % len(self.template_dict[addon])
+            if self.STATIC_INHERIT_DIRECTIVE in template_tree.attrib:
+                inherit_mode = template_tree.attrib.get(self.STATIC_INHERIT_MODE_DIRECTIVE, self.DEFAULT_MODE)
+                if inherit_mode not in [self.PRIMARY_MODE, self.EXTENSION_MODE]:
+                    raise ValueError(_("Invalid inherit mode. Module %s and template name %s") % (addon, template_name))
+
+                parent_addon, parent_name = self._get_parent_template(addon, template_tree)
+
+                # After several performance tests, we found out that deepcopy is the most efficient
+                # solution in this case (compared with copy, xpath with '.' and stringifying).
+                parent_tree = copy.deepcopy(self.template_dict[parent_addon][parent_name])
+                parent_tag = parent_tree.tag
+                # replace temporarily the parent tag so it is never the target of the inheritance
+                parent_tree.tag = 't'
+                xpaths = list(template_tree)
+                if self.debug and inherit_mode == self.EXTENSION_MODE:
+                    for xpath in xpaths:
+                        xpath.insert(0, etree.Comment(" Modified by %s from %s " % (template_name, addon)))
+                inherited_template = apply_inheritance_specs(parent_tree, xpaths)
+                inherited_template.tag = parent_tag
+
+                if inherit_mode == self.PRIMARY_MODE:  # New template_tree: A' = B(A)
+                    inherited_template.set(self.NAME_TEMPLATE_DIRECTIVE, template_name)
+                    inherited_template.set(self.STATIC_INHERIT_DIRECTIVE, template_tree.attrib[self.STATIC_INHERIT_DIRECTIVE])
+                    if self.debug:
+                        self._remove_inheritance_comments(inherited_template)
+                    self.template_dict[addon][template_name] = inherited_template
+
+                else:  # Modifies original: A = B(A)
+                    self.template_dict[parent_addon][parent_name] = inherited_template
+            else:
+                if template_name in self.template_dict[addon]:
+                    raise ValueError(_("Template %s already exists in module %s") % (template_name, addon))
+                self.template_dict[addon][template_name] = template_tree
+        return all_templates_tree
+
+    def _remove_inheritance_comments(self, inherited_template):
+        '''Remove the comments added in the template already, they come from other templates extending
+        the base of this inheritance
+
+        :param inherited_template:
+        '''
+        for comment in inherited_template.xpath('//comment()'):
+            if re.match(COMMENT_PATTERN, comment.text.strip()):
+                comment.getparent().remove(comment)
+
+    def _manifest_glob(self):
+        '''Proxy for manifest_glob
+        Usefull to make 'self' testable'''
+        return manifest_glob('qweb', self.addons, self.db)
+
+    def _read_addon_file(self, file_path):
+        """Reads the content of a file given by file_path
+        Usefull to make 'self' testable
+        :param str file_path:
+        :returns: str
+        """
+        with open(file_path, 'rb') as fp:
+            contents = fp.read()
+        return contents
+
+    def _concat_xml(self, file_dict):
+        """Concatenate xml files
+
+        :param dict(list) file_dict:
+            key: addon name
+            value: list of files for an addon
+        :returns: (concatenation_result, checksum)
+        :rtype: (str, str)
+        """
+        checksum = hashlib.new('sha1')
+        if not file_dict:
+            return '', checksum.hexdigest()
+
+        root = None
+        for addon, fnames in file_dict.items():
+            for fname in fnames:
+                contents = self._read_addon_file(fname)
+                checksum.update(contents)
+                if not self.checksum_only:
+                    xml = self._compute_xml_tree(addon, fname, contents)
+
+                    if root is None:
+                        root = etree.Element(xml.tag)
+
+        for addon in self.template_dict.values():
+            for template in addon.values():
+                root.append(template)
+
+        return etree.tostring(root, encoding='utf-8') if root is not None else '', checksum.hexdigest()
+
+    def _get_qweb_templates(self):
+        """One and only entry point that gets and evaluates static qweb templates
+
+        :rtype: (str, str)
+        """
+        files = OrderedDict([(addon, list()) for addon in self.addons])
+        [files[f[2]].append(f[0]) for f in self._manifest_glob()]
+        content, checksum = self._concat_xml(files)
+        return content, checksum
+
+    @classmethod
+    def get_qweb_templates_checksum(cls, addons, db=None, debug=False):
+        return cls(addons, db, checksum_only=True, debug=debug)._get_qweb_templates()[1]
+
+    @classmethod
+    def get_qweb_templates(cls, addons, db=None, debug=False):
+        return cls(addons, db, debug=debug)._get_qweb_templates()[0]
+
+
 #----------------------------------------------------------
 # Odoo Web web Controllers
 #----------------------------------------------------------
@@ -589,8 +746,8 @@ class WebClient(http.Controller):
 
     @http.route('/web/webclient/qweb/<string:unique>', type='http', auth="none", cors="*")
     def qweb(self, unique, mods=None, db=None):
-        files = [f[0] for f in manifest_glob('qweb', addons=mods, db=db)]
-        content, _dummy = concat_xml(files)
+        content = HomeStaticTemplateHelpers.get_qweb_templates(mods, db, debug=request.session.debug)
+
         return request.make_response(content, [
                 ('Content-Type', 'text/xml'),
                 ('Cache-Control','public, max-age=' + str(CONTENT_MAXAGE))

addons/web/models/ir_http.py

@@ -7,7 +7,7 @@ from odoo import api, models
 from odoo.http import request
 from odoo.tools import ustr
 
-from odoo.addons.web.controllers.main import concat_xml, manifest_glob, module_boot
+from odoo.addons.web.controllers.main import module_boot, HomeStaticTemplateHelpers
 
 import odoo
 
@@ -28,8 +28,7 @@ class Http(models.AbstractModel):
         user_context = request.session.get_context() if request.session.uid else {}
 
         mods = module_boot()
-        files = [f[0] for f in manifest_glob('qweb', addons=','.join(mods))]
-        _, qweb_checksum = concat_xml(files)
+        qweb_checksum = HomeStaticTemplateHelpers.get_qweb_templates_checksum(addons=mods, debug=request.session.debug)
 
         lang = user_context.get("lang")
         translations_per_module, lang_params = request.env['ir.translation'].get_translations_for_webclient(mods, lang)

addons/web/static/tests/services/notification_service_tests.js

@@ -51,8 +51,8 @@ QUnit.module('Services', {
         });
         await testUtils.nextMicrotaskTick();
         var $notification = $('body .o_notification_manager .o_notification');
-        assert.strictEqual(_.str.trim($notification.html().replace(/\s+/g, ' ')),
-            "<div class=\"toast-header\"> <span role=\"img\" aria-label=\"Notification undefined\" class=\"fa fa-2x mr-3 fa-lightbulb-o o_notification_icon\" title=\"Notification undefined\"></span> <div class=\"d-flex align-items-center mr-auto font-weight-bold o_notification_title\">a</div> </div> <div class=\"toast-body\"> <div class=\"o_notification_content\">b</div> </div>",
+        assert.strictEqual($notification.html().trim().replace(/\s+/g, ' '),
+            "<div class=\"toast-header\"> <span class=\"fa fa-2x mr-3 fa-lightbulb-o o_notification_icon\" role=\"img\" aria-label=\"Notification undefined\" title=\"Notification undefined\"></span> <div class=\"d-flex align-items-center mr-auto font-weight-bold o_notification_title\">a</div> </div> <div class=\"toast-body\"> <div class=\"o_notification_content\">b</div> </div>",
             "should display notification");
         assert.containsNone($notification, '.o_notification_close', "should not display the close button in ");
         await testUtils.nextTick();
@@ -72,8 +72,8 @@ QUnit.module('Services', {
         });
         await testUtils.nextMicrotaskTick();
         var $notification = $('body .o_notification_manager .o_notification');
-        assert.strictEqual(_.str.trim($notification.html().replace(/\s+/g, ' ')),
-            "<div class=\"toast-header\"> <span role=\"img\" aria-label=\"Notification undefined\" class=\"fa fa-2x mr-3 fa-exclamation o_notification_icon\" title=\"Notification undefined\"></span> <div class=\"d-flex align-items-center mr-auto font-weight-bold o_notification_title\">a</div> </div> <div class=\"toast-body\"> <div class=\"o_notification_content\">b</div> </div>",
+        assert.strictEqual($notification.html().trim().replace(/\s+/g, ' '),
+            "<div class=\"toast-header\"> <span class=\"fa fa-2x mr-3 fa-exclamation o_notification_icon\" role=\"img\" aria-label=\"Notification undefined\" title=\"Notification undefined\"></span> <div class=\"d-flex align-items-center mr-auto font-weight-bold o_notification_title\">a</div> </div> <div class=\"toast-body\"> <div class=\"o_notification_content\">b</div> </div>",
             "should display notification");
         view.destroy();
     });
@@ -180,8 +180,8 @@ QUnit.module('Services', {
         var $notification = $('body .o_notification_manager .o_notification');
         assert.containsOnce($notification.eq(0), '.o_notification_close',
             "should display the close button in notification");
-        assert.strictEqual(_.str.trim($notification.eq(0).html().replace(/\s+/g, ' ')),
-            "<div class=\"toast-header\"> <span role=\"img\" aria-label=\"Notification undefined\" class=\"fa fa-2x mr-3 fa-question-circle-o o_notification_icon\" title=\"Notification undefined\"></span> <div class=\"d-flex align-items-center mr-auto font-weight-bold o_notification_title\">a0</div> <button aria-label=\"Close\" class=\"mb-1 close o_notification_close\" data-dismiss=\"toast\" type=\"button\"> <span aria-hidden=\"true\" class=\"d-inline\">×</span> </button> </div> <div class=\"toast-body\"> <div class=\"o_notification_content\">b0</div> <div class=\"mt-2 o_notification_buttons\"> <button class=\"btn btn-primary\" type=\"button\"> <span>accept0</span> </button><button class=\"btn btn-secondary\" type=\"button\"> <span>refuse0</span> </button> </div> </div>",
+        assert.strictEqual($notification.html().trim().replace(/\s+/g, ' '),
+            "<div class=\"toast-header\"> <span class=\"fa fa-2x mr-3 fa-question-circle-o o_notification_icon\" role=\"img\" aria-label=\"Notification undefined\" title=\"Notification undefined\"></span> <div class=\"d-flex align-items-center mr-auto font-weight-bold o_notification_title\">a0</div> <button type=\"button\" class=\"mb-1 close o_notification_close\" data-dismiss=\"toast\" aria-label=\"Close\"> <span class=\"d-inline\" aria-hidden=\"true\">×</span> </button> </div> <div class=\"toast-body\"> <div class=\"o_notification_content\">b0</div> <div class=\"mt-2 o_notification_buttons\"> <button type=\"button\" class=\"btn btn-primary\"> <span>accept0</span> </button><button type=\"button\" class=\"btn btn-secondary\"> <span>refuse0</span> </button> </div> </div>",
             "should display notification");
 
         testUtils.dom.click($notification.find('.o_notification_buttons button:contains(accept0)'));

addons/web/tests/test_serving_base.py

@@ -2,9 +2,16 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 import random
+import re
+import unittest
+from unittest.mock import patch
+import textwrap
+from datetime import datetime, timedelta
+from lxml import etree
 
-from odoo.tests.common import BaseCase
+from odoo.tests.common import BaseCase, tagged
 from odoo.tools import topological_sort
+from odoo.addons.web.controllers.main import HomeStaticTemplateHelpers
 
 
 def sample(population):
@@ -36,3 +43,411 @@ class TestModulesLoading(BaseCase):
                     module, sorted_modules.index(module), deps, seen
                 ))
             seen.add(module)
+
+
+@tagged('standard', 'at_install', 'static_templates')
+class TestStaticInheritance(unittest.TestCase):
+
+    def setUp(self):
+        super(TestStaticInheritance, self).setUp()
+        # output is "manifest_glob" return
+        self.modules = [
+            ('module_1_file_1', None, 'module_1'),
+            ('module_2_file_1', None, 'module_2'),
+        ]
+
+        self.template_files = {
+            'module_1_file_1': b"""
+                <templates id="template" xml:space="preserve">
+                    <form t-name="template_1_1" random-attr="gloria">
+                        <div>At first I was afraid</div>
+                        <div>Kept thinking I could never live without you by my side</div>
+                    </form>
+                    <t t-name="template_1_2">
+                        <div>And I grew strong</div>
+                    </t>
+                </templates>
+                """,
+
+            'module_2_file_1': b"""
+                <templates id="template" xml:space="preserve">
+                    <form t-name="template_2_1" t-inherit="module_1.template_1_1" t-inherit-mode="primary">
+                        <xpath expr="//div[1]" position="after">
+                            <div>I was petrified</div>
+                        </xpath>
+                        <xpath expr="//div[2]" position="after">
+                            <div>But then I spent so many nights thinking how you did me wrong</div>
+                        </xpath>
+                    </form>
+                    <div t-name="template_2_2">
+                        <div>And I learned how to get along</div>
+                    </div>
+                    <form t-inherit="module_1.template_1_2" t-inherit-mode="extension">
+                        <xpath expr="//div[1]" position="after">
+                            <div>And I learned how to get along</div>
+                        </xpath>
+                    </form>
+                </templates>
+                """,
+        }
+        self._set_patchers()
+        self._toggle_patchers('start')
+        self._reg_replace_ws = r"\s|\t"
+
+    def tearDown(self):
+        super(TestStaticInheritance, self).tearDown()
+        self._toggle_patchers('stop')
+
+    # Custom Assert
+    def assertXMLEqual(self, output, expected):
+        self.assertTrue(output)
+        self.assertTrue(expected)
+        output = textwrap.dedent(output.decode('UTF-8')).strip()
+        output = re.sub(self._reg_replace_ws, '', output)
+
+        expected = textwrap.dedent(expected.decode('UTF-8')).strip()
+        expected = re.sub(self._reg_replace_ws, '', expected)
+        self.assertEqual(output, expected)
+
+    # Private methods
+    def _get_module_names(self):
+        return ','.join([glob[2] for glob in self.modules])
+
+    def _set_patchers(self):
+        def _patched_for_manifest_glob(*args, **kwargs):
+            # Ordered by module
+            return self.modules
+
+        def _patch_for_read_addon_file(*args, **kwargs):
+            return self.template_files[args[1]]
+
+        self.patchers = [
+            patch.object(HomeStaticTemplateHelpers, '_manifest_glob', _patched_for_manifest_glob),
+            patch.object(HomeStaticTemplateHelpers, '_read_addon_file', _patch_for_read_addon_file),
+        ]
+
+    def _toggle_patchers(self, mode):
+        self.assertTrue(mode in ('start', 'stop'))
+        for p in self.patchers:
+            getattr(p, mode)()
+
+    # Actual test cases
+    def test_static_inheritance_01(self):
+        contents = HomeStaticTemplateHelpers.get_qweb_templates(addons=self._get_module_names(), debug=True)
+        expected = b"""
+            <templates>
+                <form t-name="template_1_1" random-attr="gloria">
+                    <div>At first I was afraid</div>
+                    <div>Kept thinking I could never live without you by my side</div>
+                </form>
+                <t t-name="template_1_2">
+                    <div>And I grew strong</div>
+                    <!-- Modified by anonymous_template_2 from module_2 -->
+                    <div>And I learned how to get along</div>
+                </t>
+                <form t-name="template_2_1" random-attr="gloria" t-inherit="module_1.template_1_1">
+                    <div>At first I was afraid</div>
+                    <div>I was petrified</div>
+                    <div>But then I spent so many nights thinking how you did me wrong</div>
+                    <div>Kept thinking I could never live without you by my side</div>
+                </form>
+                <div t-name="template_2_2">
+                    <div>And I learned how to get along</div>
+                </div>
+            </templates>
+        """
+
+        self.assertXMLEqual(contents, expected)
+
+    def test_static_inheritance_in_same_module(self):
+        self.modules = [
+            ('module_1_file_1', None, 'module_1'),
+            ('module_1_file_2', None, 'module_1'),
+        ]
+
+        self.template_files = {
+            'module_1_file_1': b'''
+                <templates id="template" xml:space="preserve">
+                    <form t-name="template_1_1">
+                        <div>At first I was afraid</div>
+                        <div>Kept thinking I could never live without you by my side</div>
+                    </form>
+                </templates>
+            ''',
+
+            'module_1_file_2': b'''
+                <templates id="template" xml:space="preserve">
+                    <form t-name="template_1_2" t-inherit="template_1_1" t-inherit-mode="primary">
+                        <xpath expr="//div[1]" position="after">
+                            <div>I was petrified</div>
+                        </xpath>
+                    </form>
+                </templates>
+            '''
+        }
+        contents = HomeStaticTemplateHelpers.get_qweb_templates(addons=self._get_module_names(), debug=True)
+        expected = b"""
+            <templates>
+                <form t-name="template_1_1">
+                    <div>At first I was afraid</div>
+                    <div>Kept thinking I could never live without you by my side</div>
+                </form>
+                <form t-name="template_1_2" t-inherit="template_1_1">
+                    <div>At first I was afraid</div>
+                    <div>I was petrified</div>
+                    <div>Kept thinking I could never live without you by my side</div>
+                </form>
+            </templates>
+        """
+
+        self.assertXMLEqual(contents, expected)
+
+    def test_static_inheritance_in_same_file(self):
+        self.modules = [
+            ('module_1_file_1', None, 'module_1'),
+        ]
+
+        self.template_files = {
+            'module_1_file_1': b'''
+                <templates id="template" xml:space="preserve">
+                    <form t-name="template_1_1">
+                        <div>At first I was afraid</div>
+                        <div>Kept thinking I could never live without you by my side</div>
+                    </form>
+                    <form t-name="template_1_2" t-inherit="template_1_1" t-inherit-mode="primary">
+                        <xpath expr="//div[1]" position="after">
+                            <div>I was petrified</div>
+                        </xpath>
+                    </form>
+                </templates>
+            ''',
+        }
+        contents = HomeStaticTemplateHelpers.get_qweb_templates(addons=self._get_module_names(), debug=True)
+        expected = b"""
+            <templates>
+                <form t-name="template_1_1">
+                    <div>At first I was afraid</div>
+                    <div>Kept thinking I could never live without you by my side</div>
+                </form>
+                <form t-name="template_1_2" t-inherit="template_1_1">
+                    <div>At first I was afraid</div>
+                    <div>I was petrified</div>
+                    <div>Kept thinking I could never live without you by my side</div>
+                </form>
+            </templates>
+        """
+
+        self.assertXMLEqual(contents, expected)
+
+    def test_static_inherit_extended_template(self):
+        self.modules = [
+            ('module_1_file_1', None, 'module_1'),
+        ]
+        self.template_files = {
+            'module_1_file_1': b'''
+                <templates id="template" xml:space="preserve">
+                    <form t-name="template_1_1">
+                        <div>At first I was afraid</div>
+                        <div>Kept thinking I could never live without you by my side</div>
+                    </form>
+                    <form t-name="template_1_2" t-inherit="template_1_1" t-inherit-mode="extension">
+                        <xpath expr="//div[1]" position="after">
+                            <div>I was petrified</div>
+                        </xpath>
+                    </form>
+                    <form t-name="template_1_3" t-inherit="template_1_1" t-inherit-mode="primary">
+                        <xpath expr="//div[3]" position="after">
+                            <div>But then I spent so many nights thinking how you did me wrong</div>
+                        </xpath>
+                    </form>
+                </templates>
+            ''',
+        }
+        contents = HomeStaticTemplateHelpers.get_qweb_templates(addons=self._get_module_names(), debug=True)
+        expected = b"""
+            <templates>
+                <form t-name="template_1_1">
+                    <div>At first I was afraid</div>
+                    <!-- Modified by template_1_2 from module_1 -->
+                    <div>I was petrified</div>
+                    <div>Kept thinking I could never live without you by my side</div>
+                </form>
+                <form t-name="template_1_3" t-inherit="template_1_1">
+                    <div>At first I was afraid</div>
+                    <div>I was petrified</div>
+                    <div>Kept thinking I could never live without you by my side</div>
+                    <div>But then I spent so many nights thinking how you did me wrong</div>
+                </form>
+            </templates>
+        """
+
+        self.assertXMLEqual(contents, expected)
+
+    def test_sibling_extension(self):
+        self.modules = [
+            ('module_1_file_1', None, 'module_1'),
+            ('module_2_file_1', None, 'module_2'),
+            ('module_3_file_1', None, 'module_3'),
+        ]
+        self.template_files = {
+            'module_1_file_1': b'''
+                <templates id="template" xml:space="preserve">
+                    <form t-name="template_1_1">
+                        <div>I am a man of constant sorrow</div>
+                        <div>I've seen trouble all my days</div>
+                    </form>
+                </templates>
+            ''',
+
+            'module_2_file_1': b'''
+                <templates id="template" xml:space="preserve">
+                    <form t-name="template_2_1" t-inherit="module_1.template_1_1" t-inherit-mode="extension">
+                        <xpath expr="//div[1]" position="after">
+                            <div>In constant sorrow all through his days</div>
+                        </xpath>
+                    </form>
+                </templates>
+            ''',
+
+            'module_3_file_1': b'''
+                <templates id="template" xml:space="preserve">
+                    <form t-name="template_3_1" t-inherit="module_1.template_1_1" t-inherit-mode="extension">
+                        <xpath expr="//div[2]" position="after">
+                            <div>Oh Brother !</div>
+                        </xpath>
+                    </form>
+                </templates>
+            '''
+        }
+
+        contents = HomeStaticTemplateHelpers.get_qweb_templates(addons=self._get_module_names(), debug=True)
+        expected = b"""
+            <templates>
+                <form t-name="template_1_1">
+                    <div>I am a man of constant sorrow</div>
+                    <!-- Modified by template_2_1 from module_2 -->
+                    <div>In constant sorrow all through his days</div>
+                    <!-- Modified by template_3_1 from module_3 -->
+                    <div>Oh Brother !</div>
+                    <div>I've seen trouble all my days</div>
+                </form>
+            </templates>
+        """
+
+        self.assertXMLEqual(contents, expected)
+
+    def test_static_misordered_modules(self):
+        self.modules.reverse()
+        with self.assertRaises(ValueError) as ve:
+            HomeStaticTemplateHelpers.get_qweb_templates(addons=self._get_module_names(), debug=True)
+
+        self.assertEqual(
+            str(ve.exception),
+            'Module module_1 not loaded or inexistent, or templates of addon being loaded (module_2) are misordered'
+        )
+
+    def test_static_misordered_templates(self):
+        self.template_files['module_2_file_1'] = b"""
+            <templates id="template" xml:space="preserve">
+                <form t-name="template_2_1" t-inherit="module_2.template_2_2" t-inherit-mode="primary">
+                    <xpath expr="//div[1]" position="after">
+                        <div>I was petrified</div>
+                    </xpath>
+                </form>
+                <div t-name="template_2_2">
+                    <div>And I learned how to get along</div>
+                </div>
+            </templates>
+        """
+        with self.assertRaises(ValueError) as ve:
+            HomeStaticTemplateHelpers.get_qweb_templates(addons=self._get_module_names(), debug=True)
+
+        self.assertEqual(
+            str(ve.exception),
+            'No template found to inherit from. Module module_2 and template name template_2_2'
+        )
+
+    def _sick_script(self, nMod, nFilePerMod, nTemplatePerFile, stepInheritInModule=2, stepInheritPreviousModule=3):
+        """
+        Make a sick amount of templates to test perf
+        nMod modules
+        each module: has nFilesPerModule files, each of which contains nTemplatePerFile templates
+        """
+        self.modules = []
+        self.template_files = {}
+        number_templates = 0
+        for m in range(nMod):
+            for f in range(nFilePerMod):
+                mname = 'mod_%s' % m
+                fname = 'mod_%s_file_%s' % (m, f)
+                self.modules.append((fname, None, mname))
+
+                _file = '<templates id="template" xml:space="preserve">'
+
+                for t in range(nTemplatePerFile):
+                    _template = ''
+                    if t % stepInheritInModule or t % stepInheritPreviousModule or t == 0:
+                        _template += """
+                            <div t-name="template_%(t_number)s_mod_%(m_number)s">
+                                <div>Parent</div>
+                            </div>
+                        """
+
+                    elif not t % stepInheritInModule and t >= 1:
+                        _template += """
+                            <div t-name="template_%(t_number)s_mod_%(m_number)s"
+                                t-inherit="template_%(t_inherit)s_mod_%(m_number)s"
+                                t-inherit-mode="primary">
+                                <xpath expr="//div[1]" position="before">
+                                    <div>Sick XPath</div>
+                                </xpath>
+                            </div>
+                        """
+
+                    elif not t % stepInheritPreviousModule and m >= 1:
+                        _template += """
+                            <div t-name="template_%(t_number)s_mod_%(m_number)s"
+                                t-inherit="mod_%(m_module_inherit)s.template_%(t_module_inherit)s_mod_%(m_module_inherit)s"
+                                t-inherit-mode="primary">
+                                <xpath expr="//div[1]" position="inside">
+                                    <div>Mental XPath</div>
+                                </xpath>
+                            </div>
+                        """
+                    if _template:
+                        number_templates += 1
+
+                    _template_number = 1000 * f + t
+                    _file += _template % {
+                        't_number': _template_number,
+                        'm_number': m,
+                        't_inherit': _template_number - 1,
+                        't_module_inherit': _template_number,
+                        'm_module_inherit': m - 1,
+                    }
+                _file += '</templates>'
+
+                self.template_files[fname] = _file.encode()
+        self.assertEqual(number_templates, nMod * nFilePerMod * nTemplatePerFile)
+
+    def test_performance_2500(self):
+        nMod, nFilePerMod, nTemplatePerFile = 50, 5, 10
+        self._sick_script(nMod, nFilePerMod, nTemplatePerFile)
+
+        before = datetime.now()
+        contents = HomeStaticTemplateHelpers.get_qweb_templates(addons=self._get_module_names(), debug=True)
+        after = datetime.now()
+        self.assertLessEqual(after - before, timedelta(milliseconds=130))
+
+        whole_tree = etree.fromstring(contents)
+        self.assertEqual(len(whole_tree), nMod * nFilePerMod * nTemplatePerFile)
+
+    def test_performance_25000(self):
+        nMod, nFilePerMod, nTemplatePerFile = 50, 5, 100
+        self._sick_script(nMod, nFilePerMod, nTemplatePerFile)
+
+        before = datetime.now()
+        HomeStaticTemplateHelpers.get_qweb_templates(addons=self._get_module_names(), debug=True)
+        after = datetime.now()
+
+        self.assertLessEqual(after - before, timedelta(milliseconds=1000))

odoo/addons/base/models/ir_ui_view.py

@@ -1,7 +1,6 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 import collections
-import copy
 import datetime
 import fnmatch
 import logging
@@ -24,7 +23,7 @@ from odoo.exceptions import ValidationError
 from odoo.http import request
 from odoo.modules.module import get_resource_from_path, get_resource_path
 from odoo.osv import orm
-from odoo.tools import config, graph, ConstantMapping, SKIPPED_ELEMENT_TYPES, pycompat
+from odoo.tools import config, graph, ConstantMapping, pycompat, apply_inheritance_specs, locate_node
 from odoo.tools.convert import _fix_multiple_roots
 from odoo.tools.json import scriptsafe as json_scriptsafe
 from odoo.tools.safe_eval import safe_eval
@@ -133,31 +132,7 @@ def get_view_arch_from_file(filename, xmlid):
     _logger.warning("Could not find view arch definition in file '%s' for xmlid '%s'", filename, xmlid_search)
     return None
 
-def add_text_before(node, text):
-    """ Add text before ``node`` in its XML tree. """
-    if text is None:
-        return
-    prev = node.getprevious()
-    if prev is not None:
-        prev.tail = (prev.tail or "") + text
-    else:
-        parent = node.getparent()
-        parent.text = (parent.text or "") + text
 
-def add_text_inside(node, text):
-    """ Add text inside ``node``. """
-    if text is None:
-        return
-    if len(node):
-        node[-1].tail = (node[-1].tail or "") + text
-    else:
-        node.text = (node.text or "") + text
-
-def remove_element(node):
-    """ Remove ``node`` but not its tail, from its XML tree. """
-    add_text_before(node, node.tail)
-    node.tail = None
-    node.getparent().remove(node)
 
 xpath_utils = etree.FunctionNamespace(None)
 xpath_utils['hasclass'] = _hasclass
@@ -562,28 +537,7 @@ actual arch.
         :param spec: a modifying node in an inheriting view
         :return: a node in the source matching the spec
         """
-        if spec.tag == 'xpath':
-            nodes = etree.ETXPath(spec.get('expr'))(arch)
-            return nodes[0] if nodes else None
-        elif spec.tag == 'field':
-            # Only compare the field name: a field can be only once in a given view
-            # at a given level (and for multilevel expressions, we should use xpath
-            # inheritance spec anyway).
-            for node in arch.iter('field'):
-                if node.get('name') == spec.get('name'):
-                    return node
-            return None
-
-        for node in arch.iter(spec.tag):
-            if isinstance(node, SKIPPED_ELEMENT_TYPES):
-                continue
-            if all(node.get(attr) == spec.get(attr) for attr in spec.attrib
-                   if attr not in ('position', 'version')):
-                # Version spec should match parent's root element's version
-                if spec.get('version') and spec.get('version') != arch.get('version'):
-                    return None
-                return node
-        return None
+        return locate_node(arch, spec)
 
     def inherit_branding(self, specs_tree, view_id, root_id):
         for node in specs_tree.iterchildren(tag=etree.Element):
@@ -613,103 +567,10 @@ actual arch.
         """
         # Queue of specification nodes (i.e. nodes describing where and
         # changes to apply to some parent architecture).
-        specs = [specs_tree]
-
-        def extract(spec):
-            """
-            Utility function that locates a node given a specification, remove
-            it from the source and returns it.
-            """
-            if len(spec):
-                self.raise_view_error(_("Invalid specification for moved nodes: '%s'") %
-                                      etree.tostring(spec), inherit_id)
-            to_extract = self.locate_node(source, spec)
-            if to_extract is not None:
-                remove_element(to_extract)
-                return to_extract
-            else:
-                self.raise_view_error(_("Element '%s' cannot be located in parent view") %
-                                      etree.tostring(spec), inherit_id)
-
-        while len(specs):
-            spec = specs.pop(0)
-            if isinstance(spec, SKIPPED_ELEMENT_TYPES):
-                continue
-            if spec.tag == 'data':
-                specs += [c for c in spec]
-                continue
-            node = self.locate_node(source, spec)
-            if node is not None:
-                pos = spec.get('position', 'inside')
-                if pos == 'replace':
-                    for loc in spec.xpath(".//*[text()='$0']"):
-                        loc.text = ''
-                        loc.append(copy.deepcopy(node))
-                    if node.getparent() is None:
-                        source = copy.deepcopy(spec[0])
-                    else:
-                        for child in spec:
-                            if child.get('position') == 'move':
-                                child = extract(child)
-                            node.addprevious(child)
-                        node.getparent().remove(node)
-                elif pos == 'attributes':
-                    for child in spec.getiterator('attribute'):
-                        attribute = child.get('name')
-                        value = child.text or ''
-                        if child.get('add') or child.get('remove'):
-                            assert not child.text
-                            separator = child.get('separator', ',')
-                            if separator == ' ':
-                                separator = None    # squash spaces
-                            to_add = (
-                                s for s in (s.strip() for s in child.get('add', '').split(separator))
-                                if s
-                            )
-                            to_remove = {s.strip() for s in child.get('remove', '').split(separator)}
-                            values = (s.strip() for s in node.get(attribute, '').split(separator))
-                            value = (separator or ' ').join(itertools.chain(
-                                (v for v in values if v not in to_remove),
-                                to_add
-                            ))
-                        if value:
-                            node.set(attribute, value)
-                        elif attribute in node.attrib:
-                            del node.attrib[attribute]
-                elif pos == 'inside':
-                    add_text_inside(node, spec.text)
-                    for child in spec:
-                        if child.get('position') == 'move':
-                            child = extract(child)
-                        node.append(child)
-                elif pos == 'after':
-                    # add a sentinel element right after node, insert content of
-                    # spec before the sentinel, then remove the sentinel element
-                    sentinel = E.sentinel()
-                    node.addnext(sentinel)
-                    add_text_before(sentinel, spec.text)
-                    for child in spec:
-                        if child.get('position') == 'move':
-                            child = extract(child)
-                        sentinel.addprevious(child)
-                    remove_element(sentinel)
-                elif pos == 'before':
-                    add_text_before(node, spec.text)
-                    for child in spec:
-                        if child.get('position') == 'move':
-                            child = extract(child)
-                        node.addprevious(child)
-                else:
-                    self.raise_view_error(_("Invalid position attribute: '%s'") % pos, inherit_id)
-
-            else:
-                attrs = ''.join([
-                    ' %s="%s"' % (attr, spec.get(attr))
-                    for attr in spec.attrib
-                    if attr != 'position'
-                ])
-                tag = "<%s%s>" % (spec.tag, attrs)
-                self.raise_view_error(_("Element '%s' cannot be located in parent view") % tag, inherit_id)
+        try:
+            source = apply_inheritance_specs(source, specs_tree)
+        except ValueError as e:
+            self.raise_view_error(str(e), inherit_id)
 
         return source
 

odoo/tools/__init__.py

@@ -18,3 +18,4 @@ from .debugger import *
 from .xml_utils import *
 from .date_utils import *
 from .convert import *
+from .template_inheritance import *

odoo/tools/template_inheritance.py

@@ -0,0 +1,197 @@
+
+from lxml import etree
+from lxml.builder import E
+import copy
+import itertools
+
+from odoo.tools.translate import _
+from odoo.tools import SKIPPED_ELEMENT_TYPES
+
+
+def add_text_before(node, text):
+    """ Add text before ``node`` in its XML tree. """
+    if text is None:
+        return
+    prev = node.getprevious()
+    if prev is not None:
+        prev.tail = (prev.tail or "") + text
+    else:
+        parent = node.getparent()
+        parent.text = (parent.text or "") + text
+
+
+def add_text_inside(node, text):
+    """ Add text inside ``node``. """
+    if text is None:
+        return
+    if len(node):
+        node[-1].tail = (node[-1].tail or "") + text
+    else:
+        node.text = (node.text or "") + text
+
+
+def remove_element(node):
+    """ Remove ``node`` but not its tail, from its XML tree. """
+    add_text_before(node, node.tail)
+    node.tail = None
+    node.getparent().remove(node)
+
+
+def locate_node(arch, spec):
+    """ Locate a node in a source (parent) architecture.
+
+    Given a complete source (parent) architecture (i.e. the field
+    `arch` in a view), and a 'spec' node (a node in an inheriting
+    view that specifies the location in the source view of what
+    should be changed), return (if it exists) the node in the
+    source view matching the specification.
+
+    :param arch: a parent architecture to modify
+    :param spec: a modifying node in an inheriting view
+    :return: a node in the source matching the spec
+    """
+    if spec.tag == 'xpath':
+        nodes = etree.ETXPath(spec.get('expr'))(arch)
+        return nodes[0] if nodes else None
+    elif spec.tag == 'field':
+        # Only compare the field name: a field can be only once in a given view
+        # at a given level (and for multilevel expressions, we should use xpath
+        # inheritance spec anyway).
+        for node in arch.iter('field'):
+            if node.get('name') == spec.get('name'):
+                return node
+        return None
+
+    for node in arch.iter(spec.tag):
+        if isinstance(node, SKIPPED_ELEMENT_TYPES):
+            continue
+        if all(node.get(attr) == spec.get(attr) for attr in spec.attrib
+               if attr not in ('position', 'version')):
+            # Version spec should match parent's root element's version
+            if spec.get('version') and spec.get('version') != arch.get('version'):
+                return None
+            return node
+    return None
+
+
+def apply_inheritance_specs(source, specs_tree):
+    """ Apply an inheriting view (a descendant of the base view)
+
+    Apply to a source architecture all the spec nodes (i.e. nodes
+    describing where and what changes to apply to some parent
+    architecture) given by an inheriting view.
+
+    :param Element source: a parent architecture to modify
+    :return: a modified source where the specs are applied
+    :rtype: Element
+    """
+    # Queue of specification nodes (i.e. nodes describing where and
+    # changes to apply to some parent architecture).
+    specs = specs_tree if isinstance(specs_tree, list) else [specs_tree]
+
+    def extract(spec):
+        """
+        Utility function that locates a node given a specification, remove
+        it from the source and returns it.
+        """
+        if len(spec):
+            raise ValueError(
+                _("Invalid specification for moved nodes: '%s'") %
+                etree.tostring(spec)
+            )
+        to_extract = locate_node(source, spec)
+        if to_extract is not None:
+            remove_element(to_extract)
+            return to_extract
+        else:
+            raise ValueError(
+                _("Element '%s' cannot be located in parent view") %
+                etree.tostring(spec)
+            )
+
+    while len(specs):
+        spec = specs.pop(0)
+        if isinstance(spec, SKIPPED_ELEMENT_TYPES):
+            continue
+        if spec.tag == 'data':
+            specs += [c for c in spec]
+            continue
+        node = locate_node(source, spec)
+        if node is not None:
+            pos = spec.get('position', 'inside')
+            if pos == 'replace':
+                for loc in spec.xpath(".//*[text()='$0']"):
+                    loc.text = ''
+                    loc.append(copy.deepcopy(node))
+                if node.getparent() is None:
+                    source = copy.deepcopy(spec[0])
+                else:
+                    for child in spec:
+                        if child.get('position') == 'move':
+                            child = extract(child)
+                        node.addprevious(child)
+                    node.getparent().remove(node)
+            elif pos == 'attributes':
+                for child in spec.getiterator('attribute'):
+                    attribute = child.get('name')
+                    value = child.text or ''
+                    if child.get('add') or child.get('remove'):
+                        assert not child.text
+                        separator = child.get('separator', ',')
+                        if separator == ' ':
+                            separator = None    # squash spaces
+                        to_add = (
+                            s for s in (s.strip() for s in child.get('add', '').split(separator))
+                            if s
+                        )
+                        to_remove = {s.strip() for s in child.get('remove', '').split(separator)}
+                        values = (s.strip() for s in node.get(attribute, '').split(separator))
+                        value = (separator or ' ').join(itertools.chain(
+                            (v for v in values if v not in to_remove),
+                            to_add
+                        ))
+                    if value:
+                        node.set(attribute, value)
+                    elif attribute in node.attrib:
+                        del node.attrib[attribute]
+            elif pos == 'inside':
+                add_text_inside(node, spec.text)
+                for child in spec:
+                    if child.get('position') == 'move':
+                        child = extract(child)
+                    node.append(child)
+            elif pos == 'after':
+                # add a sentinel element right after node, insert content of
+                # spec before the sentinel, then remove the sentinel element
+                sentinel = E.sentinel()
+                node.addnext(sentinel)
+                add_text_before(sentinel, spec.text)
+                for child in spec:
+                    if child.get('position') == 'move':
+                        child = extract(child)
+                    sentinel.addprevious(child)
+                remove_element(sentinel)
+            elif pos == 'before':
+                add_text_before(node, spec.text)
+                for child in spec:
+                    if child.get('position') == 'move':
+                        child = extract(child)
+                    node.addprevious(child)
+            else:
+                raise ValueError(
+                    _("Invalid position attribute: '%s'") %
+                    pos
+                )
+
+        else:
+            attrs = ''.join([
+                ' %s="%s"' % (attr, spec.get(attr))
+                for attr in spec.attrib
+                if attr != 'position'
+            ])
+            tag = "<%s%s>" % (spec.tag, attrs)
+            raise ValueError(
+                _("Element '%s' cannot be located in parent view") % tag
+            )
+
+    return source