From 5594d8f191cdf040bf074a2afe08079b7a8da759 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Wed, 19 Apr 2023 08:23:35 +0000 Subject: [PATCH] [IMP] base, *: speedup assets unique computation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One of the most costly part of a page loading when the ormcache is cold is computing the assets node, the unique identifier of an attachment to validate whether the existing attachment is still valid with the current version of the static files. This operation needs to glob assets path in the filesystem, get the modification date, check attachments, ... Right now this task is not really optimized and can take some time because of an excessive number of glob on the filesystem, unnecessary exists to define absolute path, double computation of file list and modified times when getting js and css bundle separately, ... A list of modifications mainly discussed in the pr message are made with this commit to speedup things. - split css and js unique - prepare api for an in memory glob - change api to propagate absolute path and meta information through `ir.asset._get_paths`-> _get_asset_paths -> `_get_asset_content` -> `AssetsBundle` closes odoo/odoo#121159 Signed-off-by: Xavier Dollé (xdo) --- addons/test_website/tests/test_qweb.py | 29 +- addons/web/tests/test_assets_xml.py | 4 +- addons/website/models/ir_qweb.py | 4 +- odoo/addons/base/models/assetsbundle.py | 173 +++++++----- odoo/addons/base/models/ir_asset.py | 259 +++++++++--------- odoo/addons/base/models/ir_qweb.py | 30 +- .../tests/test_assetsbundle.py | 235 ++++++++++------ odoo/addons/test_lint/tests/test_pylint.py | 2 +- odoo/tools/__init__.py | 2 +- odoo/tools/constants.py | 10 + 10 files changed, 422 insertions(+), 326 deletions(-) create mode 100644 odoo/tools/constants.py diff --git a/addons/test_website/tests/test_qweb.py b/addons/test_website/tests/test_qweb.py index e498232e09a..6f165f50d6f 100644 --- a/addons/test_website/tests/test_qweb.py +++ b/addons/test_website/tests/test_qweb.py @@ -34,35 +34,42 @@ class TestQweb(TransactionCaseWithUserDemo): demo_env = self.env(user=demo) html = demo_env['ir.qweb']._render('test_website.test_template', {"user": demo}, website_id=website.id) - asset_data = etree.HTML(html).xpath('//*[@data-asset-bundle]')[0] - asset_xmlid = asset_data.attrib.get('data-asset-bundle') - asset_version = asset_data.attrib.get('data-asset-version') + asset_bundle_xmlid = 'test_website.test_bundle' + qweb = self.env['ir.qweb'] + files, _ = qweb._get_asset_content(asset_bundle_xmlid) + bundle = qweb._get_asset_bundle(asset_bundle_xmlid, files, env=self.env, css=True, js=True) + + asset_version_js = bundle.get_version('js') + asset_version_css = bundle.get_version('css') html = html.strip() html = re.sub(r'\?unique=[^"]+', '', html).encode('utf8') - attachments = demo_env['ir.attachment'].search([('url', '=like', '/web/assets/%-%/test_website.test_bundle.%')]) - self.assertEqual(len(attachments), 2) + css_attachement = demo_env['ir.attachment'].search([('url', '=like', f'/web/assets/%-{asset_version_css}/{website.id}/test_website.test_bundle.%')]) + js_attachement = demo_env['ir.attachment'].search([('url', '=like', f'/web/assets/%-{asset_version_js}/{website.id}/test_website.test_bundle.%')]) + self.assertEqual(len(css_attachement), 1) + self.assertEqual(len(js_attachement), 1) format_data = { - "js": attachments[0].url, - "css": attachments[1].url, + "css": css_attachement.url, + "js": js_attachement.url, "user_id": demo.id, "filename": "Marc%20Demo", "alt": "Marc Demo", - "asset_xmlid": asset_xmlid, - "asset_version": asset_version, + "asset_xmlid": asset_bundle_xmlid, + "asset_version_css": asset_version_css, + "asset_version_js": asset_version_js, } self.assertHTMLEqual(html, (""" - + - + diff --git a/addons/web/tests/test_assets_xml.py b/addons/web/tests/test_assets_xml.py index 5c5866c0bbd..be1f995d583 100644 --- a/addons/web/tests/test_assets_xml.py +++ b/addons/web/tests/test_assets_xml.py @@ -73,9 +73,9 @@ class TestStaticInheritanceCommon(odoo.tests.TransactionCase): 'content': None, 'media': None, }) - asset = AssetsBundle('web.test_bundle', files, env=self.env, css=False, js=True) + asset = AssetsBundle('web.test_bundle', files, env=self.env, css=False, js=True, debug='assets' if debug else '') # to_node return the files descriptions and generate attachments. - asset.to_node(css=False, js=False, debug=debug and 'assets' or '') + asset.to_node(css=False, js=False) content = asset.xml(show_inherit_info=debug) return f'\n{content}\n' diff --git a/addons/website/models/ir_qweb.py b/addons/website/models/ir_qweb.py index 26e0f3c9388..476c3f8e3c3 100644 --- a/addons/website/models/ir_qweb.py +++ b/addons/website/models/ir_qweb.py @@ -106,8 +106,8 @@ class IrQWeb(models.AbstractModel): return irQweb - def _get_asset_bundle(self, xmlid, files, env=None, css=True, js=True): - return AssetsBundleMultiWebsite(xmlid, files, env=env) + def _get_asset_bundle(self, xmlid, files, env=None, css=True, js=True, debug=False): + return AssetsBundleMultiWebsite(xmlid, files, env=env, css=css, js=js, debug=debug) def _post_processing_att(self, tagName, atts): if atts.get('data-no-post-process'): diff --git a/odoo/addons/base/models/assetsbundle.py b/odoo/addons/base/models/assetsbundle.py index 3759ad9982e..6d66755e8ec 100644 --- a/odoo/addons/base/models/assetsbundle.py +++ b/odoo/addons/base/models/assetsbundle.py @@ -114,7 +114,7 @@ class AssetsBundle(object): TRACKED_BUNDLES = ['web.assets_common', 'web.assets_backend'] - def __init__(self, name, files, env=None, css=True, js=True): + def __init__(self, name, files, env=None, css=True, js=True, debug=None): """ :param name: bundle name :param files: files to be added to the bundle @@ -131,34 +131,47 @@ class AssetsBundle(object): self.user_direction = self.env['res.lang']._lang_get( self.env.context.get('lang') or self.env.user.lang ).direction + self.has_css = css + self.has_js = js + self._checksum_cache = {} + self.is_debug_assets = debug and 'assets' in debug # asset-wide html "media" attribute for f in files: + params = { + 'url': f['url'], + 'filename': f['filename'], + 'inline': f['content'], + 'last_modified': None if self.is_debug_assets else f.get('last_modified'), + } if css: + css_params = { + 'media': f['media'], + 'direction': self.user_direction, + } if f['atype'] == 'text/sass': - self.stylesheets.append(SassStylesheetAsset(self, url=f['url'], filename=f['filename'], inline=f['content'], media=f['media'], direction=self.user_direction)) + self.stylesheets.append(SassStylesheetAsset(self, **params, **css_params)) elif f['atype'] == 'text/scss': - self.stylesheets.append(ScssStylesheetAsset(self, url=f['url'], filename=f['filename'], inline=f['content'], media=f['media'], direction=self.user_direction)) + self.stylesheets.append(ScssStylesheetAsset(self, **params, **css_params)) elif f['atype'] == 'text/less': - self.stylesheets.append(LessStylesheetAsset(self, url=f['url'], filename=f['filename'], inline=f['content'], media=f['media'], direction=self.user_direction)) + self.stylesheets.append(LessStylesheetAsset(self, **params, **css_params)) elif f['atype'] == 'text/css': - self.stylesheets.append(StylesheetAsset(self, url=f['url'], filename=f['filename'], inline=f['content'], media=f['media'], direction=self.user_direction)) + self.stylesheets.append(StylesheetAsset(self, **params, **css_params)) if js: if f['atype'] == 'text/javascript': - self.javascripts.append(JavascriptAsset(self, url=f['url'], filename=f['filename'], inline=f['content'])) + self.javascripts.append(JavascriptAsset(self, **params)) elif f['atype'] == 'text/xml': - self.templates.append(XMLAsset(self, url=f['url'], filename=f['filename'], inline=f['content'])) + self.templates.append(XMLAsset(self, **params)) - def to_node(self, css=True, js=True, debug=False, async_load=False, defer_load=False, lazy_load=False): + def to_node(self, css=True, js=True, async_load=False, defer_load=False, lazy_load=False): """ :returns [(tagName, attributes, content)] if the tag is auto close """ response = [] - is_debug_assets = debug and 'assets' in debug if css and self.stylesheets: - css_attachments = self.css(is_minified=not is_debug_assets) or [] + css_attachments = self.css(is_minified=not self.is_debug_assets) or [] for attachment in css_attachments: - if is_debug_assets: + if self.is_debug_assets: href = self.get_debug_asset_url(extra='rtl/' if self.user_direction == 'rtl' else '', name=css_attachments.name, extension='') @@ -169,7 +182,7 @@ class AssetsBundle(object): ["rel", "stylesheet"], ["href", href], ['data-asset-bundle', self.name], - ['data-asset-version', self.version], + ['data-asset-version', self.get_version('css')], ]) response.append(("link", attr, None)) if self.css_errors: @@ -187,8 +200,8 @@ class AssetsBundle(object): response.append(StylesheetAsset(self, url="/web/static/lib/bootstrap/dist/css/bootstrap.css").to_node()) if js and self.javascripts: - js_attachment = self.js(is_minified=not is_debug_assets) - src = self.get_debug_asset_url(name=js_attachment.name, extension='') if is_debug_assets else js_attachment[0].url + js_attachment = self.js(is_minified=not self.is_debug_assets) + src = self.get_debug_asset_url(name=js_attachment.name, extension='') if self.is_debug_assets else js_attachment[0].url attr = dict([ ["async", "async" if async_load else None], # lazy_load will add defer in JS otherwise this is not W3C valid @@ -197,39 +210,33 @@ class AssetsBundle(object): ["type", "text/javascript"], ["data-src" if lazy_load else "src", src], ['data-asset-bundle', self.name], - ['data-asset-version', self.version], + ['data-asset-version', self.get_version('js')], ['onerror', '__odooAssetError=1'] ]) response.append(("script", attr, None)) return response - @func.lazy_property - def last_modified_combined(self): - """Returns last modified date of linked files""" - # WebAsset are recreate here when a better solution would be to use self.stylesheets and self.javascripts - # We currently have no garanty that they are present since it will depends on js and css parameters - # last_modified is actually only usefull for the checksum and checksum should be extension specific since - # they are differents bundles. This will be a future work. + def get_version(self, asset_type): + return self.get_checksum(asset_type)[0:7] - # changing the logic from max date to combined date to fix bundle invalidation issues. - assets = [WebAsset(self, url=f['url'], filename=f['filename'], inline=f['content']) - for f in self.files - if f['atype'] in ['text/sass', "text/scss", "text/less", "text/css", "text/javascript", "text/xml"]] - return ','.join(str(asset.last_modified) for asset in assets) - - @func.lazy_property - def version(self): - return self.checksum[0:7] - - @func.lazy_property - def checksum(self): + def get_checksum(self, asset_type): """ Not really a full checksum. We compute a SHA512/256 on the rendered bundle + combined linked files last_modified date """ - check = u"%s%s" % (json.dumps(self.files, sort_keys=True), self.last_modified_combined) - return hashlib.sha512(check.encode('utf-8')).hexdigest()[:64] + if asset_type not in self._checksum_cache: + if asset_type == 'css': + assets = self.stylesheets + elif asset_type == 'js': + assets = self.javascripts + else: + raise ValueError(f'Asset type {asset_type} not known') + + unique_descriptor = ','.join(asset.unique_descriptor for asset in assets) + + self._checksum_cache[asset_type] = hashlib.sha512(unique_descriptor.encode()).hexdigest()[:64] + return self._checksum_cache[asset_type] def _get_asset_template_url(self): return "/web/assets/{id}-{unique}/{extra}{name}{sep}{extension}" @@ -277,8 +284,9 @@ class AssetsBundle(object): must exclude the current bundle. """ ira = self.env['ir.attachment'] + is_css = extension in ['css', 'min.css', 'css.map'] url = self.get_asset_url( - extra='%s' % ('rtl/' if extension in ['css', 'min.css'] and self.user_direction == 'rtl' else ''), + extra='%s' % ('rtl/' if is_css and self.user_direction == 'rtl' else ''), name=self.name, sep='', extension='.%s' % extension @@ -286,7 +294,7 @@ class AssetsBundle(object): domain = [ ('url', '=like', url), - '!', ('url', '=like', self.get_asset_url(unique=self.version)) + '!', ('url', '=like', self.get_asset_url(unique=self.get_version('css' if is_css else 'js'))) ] attachments = ira.sudo().search(domain) # avoid to invalidate cache if it's already empty (mainly useful for test) @@ -309,14 +317,15 @@ class AssetsBundle(object): :param extension: file extension (js, min.js, css) :param ignore_version: if ignore_version, the url contains a version => web/assets/%-%/name.extension (the second '%' corresponds to the version), - else: the url contains a version equal to that of the self.version - => web/assets/%-self.version/name.extension. + else: the url contains a version equal to that of the self.get_version(type) + => web/assets/%-self.get_version(type)/name.extension. """ - unique = "%" if ignore_version else self.version + is_css = extension in ['css', 'min.css', 'css.map'] + unique = "%" if ignore_version else self.get_version('css' if is_css else 'js') url_pattern = self.get_asset_url( unique=unique, - extra='%s' % ('rtl/' if extension in ['css', 'min.css'] and self.user_direction == 'rtl' else ''), + extra='%s' % ('rtl/' if is_css and self.user_direction == 'rtl' else ''), # not sure about css.map name=self.name, sep='', extension='.%s' % extension @@ -350,6 +359,7 @@ class AssetsBundle(object): # and allow to only clear the current direction bundle # (this applies to css bundles only) fname = '%s.%s' % (self.name, extension) + is_css = extension in ['css', 'min.css', 'css.map'] mimetype = ( 'text/css' if extension in ['css', 'min.css'] else 'text/xml' if extension in ['xml', 'min.xml'] else @@ -368,7 +378,7 @@ class AssetsBundle(object): attachment = ira.with_user(SUPERUSER_ID).create(values) url = self.get_asset_url( id=attachment.id, - unique=self.version, + unique=self.get_version('css' if is_css else 'js'), extra='%s' % ('rtl/' if extension in ['css', 'min.css'] and self.user_direction == 'rtl' else ''), name=fname, sep='', # included in fname @@ -390,7 +400,7 @@ class AssetsBundle(object): self.env['bus.bus']._sendone('broadcast', 'bundle_changed', { 'server_version': release.version # Needs to be dynamically imported }) - _logger.debug('Asset Changed: bundle: %s -- version: %s', self.name, self.version) + _logger.debug('Asset Changed: bundle: %s -- version: %s', self.name, self.get_version('css' if is_css else 'js')) return attachment @@ -809,12 +819,13 @@ class WebAsset(object): _ir_attach = None _id = None - def __init__(self, bundle, inline=None, url=None, filename=None): + def __init__(self, bundle, inline=None, url=None, filename=None, last_modified=None): self.bundle = bundle self.inline = inline self._filename = filename self.url = url self.html_url_args = url + self._last_modified = last_modified if not inline and not url: raise Exception("An asset should either be inlined or url linked, defined in bundle '%s'" % bundle.name) @@ -823,6 +834,10 @@ class WebAsset(object): if self._id is None: self._id = str(uuid.uuid4()) return self._id + @func.lazy_property + def unique_descriptor(self): + return f'{self.url or self.inline},{self.last_modified}' + @func.lazy_property def name(self): return '' if self.inline else self.url @@ -833,11 +848,6 @@ class WebAsset(object): def stat(self): if not (self.inline or self._filename or self._ir_attach): - path = [segment for segment in self.url.split('/') if segment] - if path and path[0] != '_custom': - self._filename = get_resource_path(*path) - if self._filename: - return try: # Test url against ir.attachments self._ir_attach = self.bundle.env['ir.attachment'].sudo()._get_serve_attachment(self.url) @@ -848,17 +858,20 @@ class WebAsset(object): def to_node(self): raise NotImplementedError() - @func.lazy_property + @property def last_modified(self): - try: - self.stat() - if self._filename: - return datetime.fromtimestamp(os.path.getmtime(self._filename)) + if self._last_modified is None: + try: + self.stat() + except Exception: # most likely nor a file or an attachment, skip it + pass + if self._filename and self.bundle.is_debug_assets: # usually _last_modified should be set exept in debug=assets + self._last_modified = os.path.getmtime(self._filename) elif self._ir_attach: - return self._ir_attach.write_date - except Exception: - pass - return datetime(1970, 1, 1) + self._last_modified = self._ir_attach.write_date.timestamp() + if not self._last_modified: + self._last_modified = -1 + return self._last_modified @property def content(self): @@ -893,11 +906,15 @@ class WebAsset(object): class JavascriptAsset(WebAsset): - def __init__(self, bundle, inline=None, url=None, filename=None): - super().__init__(bundle, inline, url, filename) + def __init__(self, bundle, **kwargs): + super().__init__(bundle, **kwargs) self._is_transpiled = None self._converted_content = None + @property + def bundle_version(self): + return self.bundle.get_version('js') + @property def is_transpiled(self): if self._is_transpiled is None: @@ -928,14 +945,14 @@ class JavascriptAsset(WebAsset): ["type", "text/javascript"], ["src", self.html_url], ['data-asset-bundle', self.bundle.name], - ['data-asset-version', self.bundle.version], + ['data-asset-version', self.bundle_version], ]), None) else: return ("script", dict([ ["type", "text/javascript"], ["charset", "utf-8"], ['data-asset-bundle', self.bundle.name], - ['data-asset-version', self.bundle.version], + ['data-asset-version', self.bundle_version], ]), self.with_header()) def with_header(self, content=None, minimal=True): @@ -967,7 +984,7 @@ class XMLAsset(WebAsset): try: content = super()._fetch_content() except AssetError as e: - return f'{json.dumps(to_text(e))}' + return f'{json.dumps(to_text(e))}' parser = etree.XMLParser(ns_clean=True, recover=True, remove_comments=True) root = etree.parse(io.BytesIO(content.encode('utf-8')), parser=parser).getroot() @@ -975,6 +992,10 @@ class XMLAsset(WebAsset): return ''.join(etree.tostring(el, encoding='unicode') for el in root) return etree.tostring(root, encoding='unicode') + @property + def bundle_version(self): + return self.bundle.get_version('js') + def to_node(self): attributes = { 'async': 'async', @@ -982,7 +1003,7 @@ class XMLAsset(WebAsset): 'type': 'text/xml', 'data-src': self.html_url, 'data-asset-bundle': self.bundle.name, - 'data-asset-version': self.bundle.version, + 'data-asset-version': self.bundle_version, } return ("script", attributes, None) @@ -1017,9 +1038,9 @@ class StylesheetAsset(WebAsset): rx_sourceMap = re.compile(r'(/\*# sourceMappingURL=.*)', re.U) rx_charset = re.compile(r'(@charset "[^"]+";)', re.U) - def __init__(self, *args, **kw): - self.media = kw.pop('media', None) - self.direction = kw.pop('direction', None) + def __init__(self, *args, media=None, direction=None, **kw): + self.media = media + self.direction = direction super().__init__(*args, **kw) if self.direction == 'rtl' and self.url: self.html_url_args = self.url.rsplit('.', 1) @@ -1030,9 +1051,19 @@ class StylesheetAsset(WebAsset): def content(self): content = super().content if self.media: - content = '@media %s { %s }' % (self.media, content) + content = '@media %s { %s }' % (self.media, content) # this is not good! return content + @property + def bundle_version(self): + return self.bundle.get_version('css') + + @func.lazy_property + def unique_descriptor(self): + direction = self.direction or '' + media = self.media or '' + return f'{self.url or self.inline},{self.last_modified},{direction},{media}' # note that media shouldn't be used in unique since it isn't in extra + def _fetch_content(self): try: content = super()._fetch_content() @@ -1081,7 +1112,7 @@ class StylesheetAsset(WebAsset): ["href", self.html_url], ["media", escape(to_text(self.media)) if self.media else None], ['data-asset-bundle', self.bundle.name], - ['data-asset-version', self.bundle.version], + ['data-asset-version', self.bundle_version], ]) return ("link", attr, None) else: @@ -1089,7 +1120,7 @@ class StylesheetAsset(WebAsset): ["type", "text/css"], ["media", escape(to_text(self.media)) if self.media else None], ['data-asset-bundle', self.bundle.name], - ['data-asset-version', self.bundle.version], + ['data-asset-version', self.bundle_version], ]) return ("style", attr, self.with_header()) diff --git a/odoo/addons/base/models/ir_asset.py b/odoo/addons/base/models/ir_asset.py index 1ad3be23e1e..3d890061ac4 100644 --- a/odoo/addons/base/models/ir_asset.py +++ b/odoo/addons/base/models/ir_asset.py @@ -1,5 +1,4 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. - import os from glob import glob from logging import getLogger @@ -9,12 +8,11 @@ import odoo import odoo.modules.module # get_manifest, don't from-import it from odoo import api, fields, models, tools from odoo.tools import misc +from odoo.tools.constants import SCRIPT_EXTENSIONS, STYLE_EXTENSIONS, TEMPLATE_EXTENSIONS, ASSET_EXTENSIONS, EXTERNAL_ASSET + _logger = getLogger(__name__) -SCRIPT_EXTENSIONS = ('js',) -STYLE_EXTENSIONS = ('css', 'scss', 'sass', 'less') -TEMPLATE_EXTENSIONS = ('xml',) DEFAULT_SEQUENCE = 16 # Directives are stored in variables for ease of use and syntax checks. @@ -27,7 +25,6 @@ REPLACE_DIRECTIVE = 'replace' INCLUDE_DIRECTIVE = 'include' # Those are the directives used with a 'target' argument/field. DIRECTIVES_WITH_TARGET = [AFTER_DIRECTIVE, BEFORE_DIRECTIVE, REPLACE_DIRECTIVE] -WILDCARD_CHARACTERS = {'*', "?", "[", "]"} def fs2web(path): @@ -36,14 +33,21 @@ def fs2web(path): return path return '/'.join(path.split(os.path.sep)) + def can_aggregate(url): parsed = urls.url_parse(url) return not parsed.scheme and not parsed.netloc and not url.startswith('/web/content') + def is_wildcard_glob(path): """Determine whether a path is a wildcarded glob eg: "/web/file[14].*" or a genuine single file path "/web/myfile.scss""" - return not WILDCARD_CHARACTERS.isdisjoint(path) + return '*' in path or '[' in path or ']' in path or '?' in path + + +def _glob_static_file(pattern): + files = glob(pattern, recursive=True) + return sorted((file, os.path.getmtime(file)) for file in files if file.rsplit('.', 1)[-1] in ASSET_EXTENSIONS) class IrAsset(models.Model): @@ -87,7 +91,7 @@ class IrAsset(models.Model): active = fields.Boolean(string='active', default=True) sequence = fields.Integer(string="Sequence", default=DEFAULT_SEQUENCE, required=True) - def _get_asset_paths(self, bundle, addons=None, css=False, js=False): + def _get_asset_paths(self, bundle, css=False, js=False): """ Fetches all asset file paths from a given list of addons matching a certain bundle. The returned list is composed of tuples containing the @@ -113,14 +117,21 @@ class IrAsset(models.Model): :returns: the list of tuples (path, addon, bundle) """ installed = self._get_installed_addons_list() - if addons is None: - addons = self._get_active_addons_list() + addons = self._get_active_addons_list() asset_paths = AssetPaths() - self._fill_asset_paths(bundle, addons, installed, css, js, asset_paths, []) + exts = [] + if js: + exts += SCRIPT_EXTENSIONS + exts += TEMPLATE_EXTENSIONS + if css: + exts += STYLE_EXTENSIONS + + addons = self._topological_sort(tuple(addons)) + self._fill_asset_paths(bundle, addons, installed, exts, asset_paths, []) return asset_paths.list - def _fill_asset_paths(self, bundle, addons, installed, css, js, asset_paths, seen): + def _fill_asset_paths(self, bundle, addons, installed, exts, asset_paths, seen): """ Fills the given AssetPaths instance by applying the operations found in the matching bundle of the given addons manifests. @@ -137,77 +148,74 @@ class IrAsset(models.Model): if bundle in seen: raise Exception("Circular assets bundle declaration: %s" % " > ".join(seen + [bundle])) - exts = [] - if js: - exts += SCRIPT_EXTENSIONS - exts += TEMPLATE_EXTENSIONS - if css: - exts += STYLE_EXTENSIONS - # this index is used for prepending: files are inserted at the beginning # of the CURRENT bundle. bundle_start_index = len(asset_paths.list) - def process_path(directive, target, path_def): - """ - This sub function is meant to take a directive and a set of - arguments and apply them to the current asset_paths list - accordingly. - - It is nested inside `_get_asset_paths` since we need the current - list of addons, extensions and asset_paths. - - :param directive: string - :param target: string or None or False - :param path_def: string - """ - if directive == INCLUDE_DIRECTIVE: - # recursively call this function for each INCLUDE_DIRECTIVE directive. - self._fill_asset_paths(path_def, addons, installed, css, js, asset_paths, seen + [bundle]) - return - - addon, paths = self._get_paths(path_def, installed, exts) - - # retrieve target index when it applies - if directive in DIRECTIVES_WITH_TARGET: - _, target_paths = self._get_paths(target, installed, exts) - if not target_paths and target.rpartition('.')[2] not in exts: - # nothing to do: the extension of the target is wrong - return - target_to_index = len(target_paths) and target_paths[0] or target - target_index = asset_paths.index(target_to_index, addon, bundle) - - if directive == APPEND_DIRECTIVE: - asset_paths.append(paths, addon, bundle) - elif directive == PREPEND_DIRECTIVE: - asset_paths.insert(paths, addon, bundle, bundle_start_index) - elif directive == AFTER_DIRECTIVE: - asset_paths.insert(paths, addon, bundle, target_index + 1) - elif directive == BEFORE_DIRECTIVE: - asset_paths.insert(paths, addon, bundle, target_index) - elif directive == REMOVE_DIRECTIVE: - asset_paths.remove(paths, addon, bundle) - elif directive == REPLACE_DIRECTIVE: - asset_paths.insert(paths, addon, bundle, target_index) - asset_paths.remove(target_paths, addon, bundle) - else: - # this should never happen - raise ValueError("Unexpected directive") - - # 1. Process the first sequence of 'ir.asset' records assets = self._get_related_assets([('bundle', '=', bundle)]).filtered('active') + # 1. Process the first sequence of 'ir.asset' records for asset in assets.filtered(lambda a: a.sequence < DEFAULT_SEQUENCE): - process_path(asset.directive, asset.target, asset.path) + self._process_path(bundle, asset.directive, asset.target, asset.path, addons, installed, exts, asset_paths, seen, bundle_start_index) # 2. Process all addons' manifests. - for addon in self._topological_sort(tuple(addons)): + for addon in addons: for command in odoo.modules.module.get_manifest(addon)['assets'].get(bundle, ()): directive, target, path_def = self._process_command(command) - process_path(directive, target, path_def) + self._process_path(bundle, directive, target, path_def, addons, installed, exts, asset_paths, seen, bundle_start_index) # 3. Process the rest of 'ir.asset' records for asset in assets.filtered(lambda a: a.sequence >= DEFAULT_SEQUENCE): - process_path(asset.directive, asset.target, asset.path) + self._process_path(bundle, asset.directive, asset.target, asset.path, addons, installed, exts, asset_paths, seen, bundle_start_index) + + def _process_path(self, bundle, directive, target, path_def, addons, installed, exts, asset_paths, seen, bundle_start_index): + """ + This sub function is meant to take a directive and a set of + arguments and apply them to the current asset_paths list + accordingly. + + It is nested inside `_get_asset_paths` since we need the current + list of addons, extensions and asset_paths. + + :param directive: string + :param target: string or None or False + :param path_def: string + """ + if directive == INCLUDE_DIRECTIVE: + # recursively call this function for each INCLUDE_DIRECTIVE directive. + self._fill_asset_paths(path_def, addons, installed, exts, asset_paths, seen + [bundle]) + return + + if can_aggregate(path_def): + paths = self._get_paths(path_def, installed, exts) + else: + paths = [(path_def, EXTERNAL_ASSET, -1)] # external urls + + # retrieve target index when it applies + if directive in DIRECTIVES_WITH_TARGET: + target_paths = self._get_paths(target, installed, exts) + if not target_paths and target.rpartition('.')[2] not in exts: + # nothing to do: the extension of the target is wrong + return + if target_paths: + target = target_paths[0][0] + target_index = asset_paths.index(target, bundle) + + if directive == APPEND_DIRECTIVE: + asset_paths.append(paths, bundle) + elif directive == PREPEND_DIRECTIVE: + asset_paths.insert(paths, bundle, bundle_start_index) + elif directive == AFTER_DIRECTIVE: + asset_paths.insert(paths, bundle, target_index + 1) + elif directive == BEFORE_DIRECTIVE: + asset_paths.insert(paths, bundle, target_index) + elif directive == REMOVE_DIRECTIVE: + asset_paths.remove(paths, bundle) + elif directive == REPLACE_DIRECTIVE: + asset_paths.insert(paths, bundle, target_index) + asset_paths.remove(target_paths, bundle) + else: + # this should never happen + raise ValueError("Unexpected directive") def _get_related_assets(self, domain): """ @@ -216,6 +224,8 @@ class IrAsset(models.Model): :param domain: search domain :returns: ir.asset recordset """ + # active_test is needed to disable some assets through filter_duplicate for website + # they will be filtered on active afterward return self.with_context(active_test=False).sudo().search(domain, order='sequence, id') def _get_related_bundle(self, target_path_def, root_bundle): @@ -231,14 +241,14 @@ class IrAsset(models.Model): """ ext = target_path_def.split('.')[-1] installed = self._get_installed_addons_list() - target_path = self._get_paths(target_path_def, installed)[1][0] + target_path, _full_path, _modified = self._get_paths(target_path_def, installed)[0] css = ext in STYLE_EXTENSIONS js = ext in SCRIPT_EXTENSIONS or ext in TEMPLATE_EXTENSIONS asset_paths = self._get_asset_paths(root_bundle, css=css, js=js) - for path, _, bundle in asset_paths: + for path, _full_path, bundle, _modified in asset_paths: if path == target_path: return bundle @@ -285,21 +295,29 @@ class IrAsset(models.Model): def _get_paths(self, path_def, installed, extensions=None): """ - Returns a list of file paths matching a given glob (path_def) as well as - the addon targeted by the path definition. If no file matches that glob, - the path definition is returned as is. This is either because the path is - not correctly written or because it points to a URL. + Returns a list of tuple (path, full_path, modified) matching a given glob (path_def). + The glob can only occur in the static direcory of an installed addon. + + If the path_def matches a (list of) file, the result will contain the full_path + and the modified time. + Ex: ('/base/static/file.js', '/home/user/source/odoo/odoo/addons/base/static/file.js', 643636800) + + If the path_def looks like a non aggregable path (http://, /web/assets), only return the path + Ex: ('http://example.com/lib.js', None, -1) + The timestamp -1 is given to be thruthy while carrying no information. + + If the path_def is not a wildward, but may still be a valid addons path, return a False path + with No timetamp + Ex: ('/_custom/web.asset_frontend', False, None) :param path_def: the definition (glob) of file paths to match :param installed: the list of installed addons :param extensions: a list of extensions that found files must match - :returns: a tuple: the addon targeted by the path definition [0] and the - list of file paths matching the definition [1] (or the glob itself if - none). Note that these paths are filtered on the given `extensions`. + :returns: a list of tuple: (path, full_path, modified) """ - paths = [] - path_url = fs2web(path_def) - path_parts = [part for part in path_url.split('/') if part] + paths = None + path_def = fs2web(path_def) # we expect to have all path definition unix style or url style, this is a safety + path_parts = [part for part in path_def.split('/') if part] addon = path_parts[0] addon_manifest = odoo.modules.module.get_manifest(addon) @@ -307,49 +325,28 @@ class IrAsset(models.Model): if addon_manifest: if addon not in installed: # Assert that the path is in the installed addons - raise Exception("Unallowed to fetch files from addon %s" % addon) - addons_path = os.path.join(addon_manifest['addons_path'], '')[:-1] - full_path = os.path.normpath(os.path.join(addons_path, *path_parts)) - - # first security layer: forbid escape from the current addon + raise Exception(f"Unallowed to fetch files from addon {addon} for file {path_def}") + addons_path = addon_manifest['addons_path'] + full_path = os.path.normpath(os.sep.join([addons_path, *path_parts])) + # forbid escape from the current addon # "/mymodule/../myothermodule" is forbidden - # the condition after the or is to further guarantee that we won't access - # a directory that happens to be named like an addon (web....) - if addon not in full_path or addons_path not in full_path: - addon = None - safe_path = False - else: + static_prefix = os.sep.join([addons_path, addon, 'static', '']) + if full_path.startswith(static_prefix): + paths_with_timestamps = _glob_static_file(full_path) paths = [ - path for path in sorted(glob(full_path, recursive=True)) + (fs2web(absolute_path[len(addons_path):]), absolute_path, timestamp) + for absolute_path, timestamp in paths_with_timestamps ] - - # second security layer: do we have the right to access the files - # that are grabbed by the glob ? - # In particular we don't want to expose data in xmls of the module - def is_safe_path(path): - try: - misc.file_path(path, SCRIPT_EXTENSIONS + STYLE_EXTENSIONS + TEMPLATE_EXTENSIONS) - except (ValueError, FileNotFoundError): - return False - if path.rpartition('.')[2] in TEMPLATE_EXTENSIONS: - # normpath will strip the trailing /, which is why it has to be added afterwards - static_path = os.path.normpath("%s/static" % addon) + os.path.sep - # Forbid xml to leak - return static_path in path - return True - - len_paths = len(paths) - paths = list(filter(is_safe_path, paths)) - safe_path = safe_path and len_paths == len(paths) - - # Web assets must be loaded using relative paths. - paths = [fs2web(path[len(addons_path):]) for path in paths] + else: + safe_path = False else: - addon = None + safe_path = False - if not paths and (not can_aggregate(path_url) or (safe_path and not is_wildcard_glob(path_url))): - # No file matching the path; the path_def could be a url. - paths = [path_url] + if not paths and not can_aggregate(path_def): # http:// or /web/content + paths = [(path_def, EXTERNAL_ASSET, -1)] + + if not paths and not is_wildcard_glob(path_def): # an attachment url most likely + paths = [(path_def, None, None)] if not paths: msg = f'IrAsset: the path "{path_def}" did not resolve to anything.' @@ -357,10 +354,10 @@ class IrAsset(models.Model): msg += " It may be due to security reasons." _logger.warning(msg) # Paths are filtered on the extensions (if any). - return addon, [ + return [ path for path in paths - if not extensions or path.split('.')[-1] in extensions + if not extensions or path[0].rsplit('.', maxsplit=1)[-1] in extensions ] def _process_command(self, command): @@ -382,7 +379,7 @@ class AssetPaths: self.list = [] self.memo = set() - def index(self, path, addon, bundle): + def index(self, path, bundle): """Returns the index of the given path in the current assets list.""" if path not in self.memo: self._raise_not_found(path, bundle) @@ -390,32 +387,32 @@ class AssetPaths: if asset[0] == path: return index - def append(self, paths, addon, bundle): + def append(self, paths, bundle): """Appends the given paths to the current list.""" - for path in paths: + for path, full_path, last_modified in paths: if path not in self.memo: - self.list.append((path, addon, bundle)) + self.list.append((path, full_path, bundle, last_modified)) self.memo.add(path) - def insert(self, paths, addon, bundle, index): + def insert(self, paths, bundle, index): """Inserts the given paths to the current list at the given position.""" to_insert = [] - for path in paths: + for path, full_path, last_modified in paths: if path not in self.memo: - to_insert.append((path, addon, bundle)) + to_insert.append((path, full_path, bundle, last_modified)) self.memo.add(path) self.list[index:index] = to_insert - def remove(self, paths_to_remove, addon, bundle): + def remove(self, paths_to_remove, bundle): """Removes the given paths from the current list.""" - paths = {path for path in paths_to_remove if path in self.memo} + paths = {path for path, _full_path, _last_modified in paths_to_remove if path in self.memo} if paths: self.list[:] = [asset for asset in self.list if asset[0] not in paths] self.memo.difference_update(paths) return if paths_to_remove: - self._raise_not_found(paths_to_remove, bundle) + self._raise_not_found([path for path, _full_path, _last_modified in paths_to_remove], bundle) def _raise_not_found(self, path, bundle): raise ValueError("File(s) %s not found in bundle %s" % (path, bundle)) diff --git a/odoo/addons/base/models/ir_qweb.py b/odoo/addons/base/models/ir_qweb.py index 8b7a3e4e155..624dba6c375 100644 --- a/odoo/addons/base/models/ir_qweb.py +++ b/odoo/addons/base/models/ir_qweb.py @@ -379,18 +379,18 @@ from dateutil.relativedelta import relativedelta from psycopg2.extensions import TransactionRollbackError from odoo import api, models, tools -from odoo.tools import config, safe_eval, pycompat, SUPPORTED_DEBUGGER +from odoo.tools import config, safe_eval, pycompat +from odoo.tools.constants import SUPPORTED_DEBUGGER, EXTERNAL_ASSET from odoo.tools.safe_eval import assert_valid_codeobj, _BUILTINS, to_opcodes, _EXPR_OPCODES, _BLACKLIST from odoo.tools.json import scriptsafe from odoo.tools.misc import str2bool from odoo.tools.image import image_data_uri from odoo.http import request -from odoo.modules.module import get_resource_path, get_module_path from odoo.tools.profiler import QwebTracker from odoo.exceptions import UserError, AccessDenied, AccessError, MissingError, ValidationError from odoo.addons.base.models.assetsbundle import AssetsBundle -from odoo.addons.base.models.ir_asset import can_aggregate, STYLE_EXTENSIONS, SCRIPT_EXTENSIONS, TEMPLATE_EXTENSIONS +from odoo.tools.constants import SCRIPT_EXTENSIONS, STYLE_EXTENSIONS, TEMPLATE_EXTENSIONS _logger = logging.getLogger(__name__) @@ -2472,22 +2472,16 @@ class IrQWeb(models.AbstractModel): @tools.ormcache('bundle', 'defer_load', 'lazy_load', 'media', 'tuple(self.env.context.get(k) for k in self._get_template_cache_keys())') def _get_asset_content(self, bundle, defer_load=False, lazy_load=False, media=None): asset_paths = self.env['ir.asset']._get_asset_paths(bundle=bundle, css=True, js=True) - files = [] remains = [] - for path, *_ in asset_paths: - ext = path.split('.')[-1] + for path, full_path, _bundle, last_modified in asset_paths: + ext = path.rpartition('.')[2] is_js = ext in SCRIPT_EXTENSIONS is_xml = ext in TEMPLATE_EXTENSIONS is_css = ext in STYLE_EXTENSIONS if not is_js and not is_xml and not is_css: continue - if is_xml: - base = get_module_path(bundle.split('.')[0]).rsplit('/', 1)[0] - if path.startswith(base): - path = path[len(base):] - mimetype = None if is_js: mimetype = 'text/javascript' @@ -2496,14 +2490,14 @@ class IrQWeb(models.AbstractModel): elif is_xml: mimetype = 'text/xml' - if can_aggregate(path): - segments = [segment for segment in path.split('/') if segment] + if full_path is not EXTERNAL_ASSET: files.append({ 'atype': mimetype, 'url': path, - 'filename': get_resource_path(*segments) if segments and segments[0] != '_custom' else None, + 'filename': full_path, 'content': '', 'media': media, + 'last_modified': last_modified, }) else: if is_js: @@ -2536,14 +2530,14 @@ class IrQWeb(models.AbstractModel): return (files, remains) - def _get_asset_bundle(self, bundle_name, files, env=None, css=True, js=True): - return AssetsBundle(bundle_name, files, env=env, css=css, js=js) + def _get_asset_bundle(self, bundle_name, files, env=None, css=True, js=True, debug=False): + return AssetsBundle(bundle_name, files, env=env, css=css, js=js, debug=debug) def _generate_asset_nodes(self, bundle, css=True, js=True, debug=False, async_load=False, defer_load=False, lazy_load=False, media=None): files, remains = self._get_asset_content(bundle, defer_load=defer_load, lazy_load=lazy_load, media=css and media or None) - asset = self._get_asset_bundle(bundle, files, env=self.env, css=css, js=js) + asset = self._get_asset_bundle(bundle, files, env=self.env, css=css, js=js, debug=debug) remains = [node for node in remains if (css and node[0] == 'link') or (js and node[0] == 'script')] - return remains + asset.to_node(css=css, js=js, debug=debug, async_load=async_load, defer_load=defer_load, lazy_load=lazy_load) + return remains + asset.to_node(css=css, js=js, async_load=async_load, defer_load=defer_load, lazy_load=lazy_load) def _get_asset_link_urls(self, bundle, debug=False): asset_nodes = self._get_asset_nodes(bundle, js=False, debug=debug) diff --git a/odoo/addons/test_assetsbundle/tests/test_assetsbundle.py b/odoo/addons/test_assetsbundle/tests/test_assetsbundle.py index 5c8a96ca0aa..1217dace5c6 100644 --- a/odoo/addons/test_assetsbundle/tests/test_assetsbundle.py +++ b/odoo/addons/test_assetsbundle/tests/test_assetsbundle.py @@ -33,50 +33,68 @@ class TestAddonPaths(TransactionCase): asset_paths = AssetPaths() self.assertFalse(asset_paths.list) - asset_paths.append(['a', 'c', 'd'], 'module1', 'bundle1') + asset_paths.append([ + ('/home/user/odoo/addons/web/a', '/web/a', 1), + ('/home/user/odoo/addons/web/c', '/web/c', 1), + ('/home/user/odoo/addons/web/d', '/web/d', 1), + ], 'bundle1') self.assertEqual(asset_paths.list, [ - ('a', 'module1', 'bundle1'), - ('c', 'module1', 'bundle1'), - ('d', 'module1', 'bundle1'), + ('/home/user/odoo/addons/web/a', '/web/a', 'bundle1', 1), + ('/home/user/odoo/addons/web/c', '/web/c', 'bundle1', 1), + ('/home/user/odoo/addons/web/d', '/web/d', 'bundle1', 1), ]) # append with a duplicate of 'c' - asset_paths.append(['c', 'f'], 'module2', 'bundle2') + asset_paths.append([ + ('/home/user/odoo/addons/web/c', '/web/c', 1), + ('/home/user/odoo/addons/web/f', '/web/f', 1), + ], 'bundle2') self.assertEqual(asset_paths.list, [ - ('a', 'module1', 'bundle1'), - ('c', 'module1', 'bundle1'), - ('d', 'module1', 'bundle1'), - ('f', 'module2', 'bundle2'), + ('/home/user/odoo/addons/web/a', '/web/a', 'bundle1', 1), + ('/home/user/odoo/addons/web/c', '/web/c', 'bundle1', 1), + ('/home/user/odoo/addons/web/d', '/web/d', 'bundle1', 1), + ('/home/user/odoo/addons/web/f', '/web/f', 'bundle2', 1), ]) # insert with a duplicate of 'c' after 'c' - asset_paths.insert(['c', 'e'], 'module3', 'bundle3', 3) + asset_paths.insert([ + ('/home/user/odoo/addons/web/c', '/web/c', 1), + ('/home/user/odoo/addons/web/e', '/web/e', 1), + ], 'bundle3', 3) self.assertEqual(asset_paths.list, [ - ('a', 'module1', 'bundle1'), - ('c', 'module1', 'bundle1'), - ('d', 'module1', 'bundle1'), - ('e', 'module3', 'bundle3'), - ('f', 'module2', 'bundle2'), + ('/home/user/odoo/addons/web/a', '/web/a', 'bundle1', 1), + ('/home/user/odoo/addons/web/c', '/web/c', 'bundle1', 1), + ('/home/user/odoo/addons/web/d', '/web/d', 'bundle1', 1), + ('/home/user/odoo/addons/web/e', '/web/e', 'bundle3', 1), + ('/home/user/odoo/addons/web/f', '/web/f', 'bundle2', 1), ]) # insert with a duplicate of 'd' before 'd' - asset_paths.insert(['b', 'd'], 'module4', 'bundle4', 1) + asset_paths.insert([ + ('/home/user/odoo/addons/web/b', '/web/b', 1), + ('/home/user/odoo/addons/web/d', '/web/d', 1) + ], 'bundle4', 1) self.assertEqual(asset_paths.list, [ - ('a', 'module1', 'bundle1'), - ('b', 'module4', 'bundle4'), - ('c', 'module1', 'bundle1'), - ('d', 'module1', 'bundle1'), - ('e', 'module3', 'bundle3'), - ('f', 'module2', 'bundle2'), + + ('/home/user/odoo/addons/web/a', '/web/a', 'bundle1', 1), + ('/home/user/odoo/addons/web/b', '/web/b', 'bundle4', 1), + ('/home/user/odoo/addons/web/c', '/web/c', 'bundle1', 1), + ('/home/user/odoo/addons/web/d', '/web/d', 'bundle1', 1), + ('/home/user/odoo/addons/web/e', '/web/e', 'bundle3', 1), + ('/home/user/odoo/addons/web/f', '/web/f', 'bundle2', 1), ]) # remove - asset_paths.remove(['c', 'd', 'g'], 'module5', 'bundle5') + asset_paths.remove([ + ('/home/user/odoo/addons/web/c', '/web/c', 1), + ('/home/user/odoo/addons/web/d', '/web/d', 1), + ('/home/user/odoo/addons/web/g', '/web/g', 1) + ], 'bundle5') self.assertEqual(asset_paths.list, [ - ('a', 'module1', 'bundle1'), - ('b', 'module4', 'bundle4'), - ('e', 'module3', 'bundle3'), - ('f', 'module2', 'bundle2'), + ('/home/user/odoo/addons/web/a', '/web/a', 'bundle1', 1), + ('/home/user/odoo/addons/web/b', '/web/b', 'bundle4', 1), + ('/home/user/odoo/addons/web/e', '/web/e', 'bundle3', 1), + ('/home/user/odoo/addons/web/f', '/web/f', 'bundle2', 1), ]) @@ -106,16 +124,22 @@ class FileTouchable(AddonManifestPatched): class TestJavascriptAssetsBundle(FileTouchable): + @classmethod + def setUpClass(cls): + super().setUpClass() + # this is mainly to avoid tests breaking when executed after pre-generate + cls.env['ir.attachment'].search([('url', '=like', '/web/assets/%test_assetsbundle%')]).unlink() + def setUp(self): super(TestJavascriptAssetsBundle, self).setUp() self.jsbundle_name = 'test_assetsbundle.bundle1' self.cssbundle_name = 'test_assetsbundle.bundle2' self.env['res.lang']._activate_lang('ar_SY') - def _get_asset(self, bundle, env=None): + def _get_asset(self, bundle, env=None, debug=''): env = (env or self.env) files, _ = env['ir.qweb']._get_asset_content(bundle) - return AssetsBundle(bundle, files, env=env) + return AssetsBundle(bundle, files, env=env, debug=debug) def _any_ira_for_bundle(self, extension, lang=None): """ Returns all ir.attachments associated to a bundle, regardless of the verion. @@ -180,7 +204,7 @@ class TestJavascriptAssetsBundle(FileTouchable): self.assertEqual(len(self._any_ira_for_bundle('min.js')), 1, "there should be one minified attachment associated to this bundle") - version0 = bundle0.version + version0 = bundle0.get_version('js') ira0 = self._any_ira_for_bundle('min.js') date0 = ira0.create_date @@ -190,7 +214,7 @@ class TestJavascriptAssetsBundle(FileTouchable): self.assertEqual(len(self._any_ira_for_bundle('min.js')), 1, "there should be one minified attachment associated to this bundle") - version1 = bundle1.version + version1 = bundle1.get_version('js') ira1 = self._any_ira_for_bundle('min.js') date1 = ira1.create_date @@ -202,18 +226,18 @@ class TestJavascriptAssetsBundle(FileTouchable): def test_03_date_invalidation(self): """ Checks that a bundle is invalidated when one of its assets' modification date is changed. """ - bundle0 = self._get_asset(self.jsbundle_name) + bundle0 = self._get_asset(self.jsbundle_name, debug="assets") bundle0.js() - last_modified0 = bundle0.last_modified_combined - version0 = bundle0.version + last_modified0 = bundle0.get_checksum('js') + version0 = bundle0.get_version('js') path = get_resource_path('test_assetsbundle', 'static', 'src', 'js', 'test_jsfile1.js') - bundle1 = self._get_asset(self.jsbundle_name) + bundle1 = self._get_asset(self.jsbundle_name, debug="assets") with self._touch(path): bundle1.js() - last_modified1 = bundle1.last_modified_combined - version1 = bundle1.version + last_modified1 = bundle1.get_checksum('js') + version1 = bundle1.get_version('js') self.assertNotEqual(last_modified0, last_modified1, "the creation date of the ir.attachment should change because the bundle has changed.") self.assertNotEqual(version0, version1, @@ -230,7 +254,7 @@ class TestJavascriptAssetsBundle(FileTouchable): bundle0 = self._get_asset(self.jsbundle_name) bundle0.js() files0 = bundle0.files - version0 = bundle0.version + version0 = bundle0.get_version('js') self.assertEqual(len(self._any_ira_for_bundle('min.js')), 1, "there should be one minified attachment associated to this bundle") @@ -244,7 +268,7 @@ class TestJavascriptAssetsBundle(FileTouchable): bundle1 = self._get_asset(self.jsbundle_name) bundle1.js() files1 = bundle1.files - version1 = bundle1.version + version1 = bundle1.get_version('js') self.assertNotEqual(files0, files1, "the list of files should be different because a file has been added to the bundle") @@ -277,8 +301,8 @@ class TestJavascriptAssetsBundle(FileTouchable): """ Checks that a bundle rendered in debug 1 mode outputs non-minified assets and create an non-minified ir.attachment. """ - debug_bundle = self._get_asset(self.jsbundle_name) - nodes = debug_bundle.to_node(debug='1') + debug_bundle = self._get_asset(self.jsbundle_name, debug='1') + nodes = debug_bundle.to_node() content = self._node_to_list(nodes) # there should be a minified file self.assertEqual(content[3].count('test_assetsbundle.bundle1.min.js'), 1, @@ -296,8 +320,8 @@ class TestJavascriptAssetsBundle(FileTouchable): """ Checks that a bundle rendered in debug assets mode outputs non-minified assets and create an non-minified ir.attachment at the . """ - debug_bundle = self._get_asset(self.jsbundle_name) - nodes = debug_bundle.to_node(debug='assets') + debug_bundle = self._get_asset(self.jsbundle_name, debug='assets') + nodes = debug_bundle.to_node() content = self._node_to_list(nodes) # there should be a non-minified file (not .min.js) self.assertEqual(content[3].count('test_assetsbundle.bundle1.js'), 1, @@ -327,7 +351,7 @@ class TestJavascriptAssetsBundle(FileTouchable): self.assertEqual(len(self._any_ira_for_bundle('min.css')), 1) - version0 = bundle0.version + version0 = bundle0.get_version('css') ira0 = self._any_ira_for_bundle('min.css') date0 = ira0.create_date @@ -336,7 +360,7 @@ class TestJavascriptAssetsBundle(FileTouchable): self.assertEqual(len(self._any_ira_for_bundle('min.css')), 1) - version1 = bundle1.version + version1 = bundle1.get_version('css') ira1 = self._any_ira_for_bundle('min.css') date1 = ira1.create_date @@ -350,7 +374,7 @@ class TestJavascriptAssetsBundle(FileTouchable): bundle0 = self._get_asset(self.cssbundle_name) bundle0.css() files0 = bundle0.files - version0 = bundle0.version + version0 = bundle0.get_version('css') self.assertEqual(len(self._any_ira_for_bundle('min.css')), 1) @@ -363,7 +387,7 @@ class TestJavascriptAssetsBundle(FileTouchable): bundle1 = self._get_asset(self.cssbundle_name) bundle1.css() files1 = bundle1.files - version1 = bundle1.version + version1 = bundle1.get_version('css') self.assertNotEqual(files0, files1) self.assertNotEqual(version0, version1) @@ -374,8 +398,8 @@ class TestJavascriptAssetsBundle(FileTouchable): def test_12_css_debug(self): """ Check that a bundle in debug mode outputs non-minified assets. """ - debug_bundle = self._get_asset(self.cssbundle_name) - nodes = debug_bundle.to_node(debug='assets') + debug_bundle = self._get_asset(self.cssbundle_name, debug='assets') + nodes = debug_bundle.to_node() content = self._node_to_list(nodes) # find back one of the original asset file self.assertIn('/web/assets/debug/test_assetsbundle.bundle2.css', content) @@ -433,7 +457,7 @@ class TestJavascriptAssetsBundle(FileTouchable): self.assertEqual(len(self._any_ira_for_bundle('min.css')), 1) - ltr_version0 = ltr_bundle0.version + ltr_version0 = ltr_bundle0.get_version('css') ltr_ira0 = self._any_ira_for_bundle('min.css') ltr_date0 = ltr_ira0.create_date @@ -442,7 +466,7 @@ class TestJavascriptAssetsBundle(FileTouchable): self.assertEqual(len(self._any_ira_for_bundle('min.css')), 1) - ltr_version1 = ltr_bundle1.version + ltr_version1 = ltr_bundle1.get_version('css') ltr_ira1 = self._any_ira_for_bundle('min.css') ltr_date1 = ltr_ira1.create_date @@ -455,7 +479,7 @@ class TestJavascriptAssetsBundle(FileTouchable): self.assertEqual(len(self._any_ira_for_bundle('min.css', lang='ar_SY')), 1) - rtl_version0 = rtl_bundle0.version + rtl_version0 = rtl_bundle0.get_version('css') rtl_ira0 = self._any_ira_for_bundle('min.css', lang='ar_SY') rtl_date0 = rtl_ira0.create_date @@ -464,7 +488,7 @@ class TestJavascriptAssetsBundle(FileTouchable): self.assertEqual(len(self._any_ira_for_bundle('min.css', lang='ar_SY')), 1) - rtl_version1 = rtl_bundle1.version + rtl_version1 = rtl_bundle1.get_version('css') rtl_ira1 = self._any_ira_for_bundle('min.css', lang='ar_SY') rtl_date1 = rtl_ira1.create_date @@ -484,35 +508,35 @@ class TestJavascriptAssetsBundle(FileTouchable): """ Checks that both css bundles are invalidated when one of its assets' modification date is changed """ # Assets access for en_US language - ltr_bundle0 = self._get_asset(self.cssbundle_name) + ltr_bundle0 = self._get_asset(self.cssbundle_name, debug='assets') ltr_bundle0.css() - ltr_last_modified0 = ltr_bundle0.last_modified_combined - ltr_version0 = ltr_bundle0.version + ltr_last_modified0 = ltr_bundle0.get_checksum('css') + ltr_version0 = ltr_bundle0.get_version('css') # Assets access for ar_SY language - rtl_bundle0 = self._get_asset(self.cssbundle_name, env=self.env(context={'lang': 'ar_SY'})) + rtl_bundle0 = self._get_asset(self.cssbundle_name, env=self.env(context={'lang': 'ar_SY'}), debug='assets') rtl_bundle0.css() - rtl_last_modified0 = rtl_bundle0.last_modified_combined - rtl_version0 = rtl_bundle0.version + rtl_last_modified0 = rtl_bundle0.get_checksum('css') + rtl_version0 = rtl_bundle0.get_version('css') # Touch test_cssfile1.css # Note: No lang specific context given while calling _get_asset so it will load assets for en_US path = get_resource_path('test_assetsbundle', 'static', 'src', 'css', 'test_cssfile1.css') - ltr_bundle1 = self._get_asset(self.cssbundle_name) + ltr_bundle1 = self._get_asset(self.cssbundle_name, debug='assets') with self._touch(path): ltr_bundle1.css() - ltr_last_modified1 = ltr_bundle1.last_modified_combined - ltr_version1 = ltr_bundle1.version + ltr_last_modified1 = ltr_bundle1.get_checksum('css') + ltr_version1 = ltr_bundle1.get_version('css') ltr_ira1 = self._any_ira_for_bundle('min.css') self.assertNotEqual(ltr_last_modified0, ltr_last_modified1) self.assertNotEqual(ltr_version0, ltr_version1) - rtl_bundle1 = self._get_asset(self.cssbundle_name, env=self.env(context={'lang': 'ar_SY'})) + rtl_bundle1 = self._get_asset(self.cssbundle_name, env=self.env(context={'lang': 'ar_SY'}), debug='assets') rtl_bundle1.css() - rtl_last_modified1 = rtl_bundle1.last_modified_combined - rtl_version1 = rtl_bundle1.version + rtl_last_modified1 = rtl_bundle1.get_checksum('css') + rtl_version1 = rtl_bundle1.get_version('css') rtl_ira1 = self._any_ira_for_bundle('min.css', lang='ar_SY') self.assertNotEqual(rtl_last_modified0, rtl_last_modified1) self.assertNotEqual(rtl_version0, rtl_version1) @@ -534,12 +558,12 @@ class TestJavascriptAssetsBundle(FileTouchable): ltr_bundle0 = self._get_asset(self.cssbundle_name) ltr_bundle0.css() ltr_files0 = ltr_bundle0.files - ltr_version0 = ltr_bundle0.version + ltr_version0 = ltr_bundle0.get_version('css') rtl_bundle0 = self._get_asset(self.cssbundle_name, env=self.env(context={'lang': 'ar_SY'})) rtl_bundle0.css() rtl_files0 = rtl_bundle0.files - rtl_version0 = rtl_bundle0.version + rtl_version0 = rtl_bundle0.get_version('css') css_bundles = self.env['ir.attachment'].search([ ('url', '=like', '/web/assets/%-%/{0}%.{1}'.format(self.cssbundle_name, 'min.css')) @@ -555,7 +579,7 @@ class TestJavascriptAssetsBundle(FileTouchable): ltr_bundle1 = self._get_asset(self.cssbundle_name) ltr_bundle1.css() ltr_files1 = ltr_bundle1.files - ltr_version1 = ltr_bundle1.version + ltr_version1 = ltr_bundle1.get_version('css') ltr_ira1 = self._any_ira_for_bundle('min.css') self.assertNotEqual(ltr_files0, ltr_files1) @@ -564,7 +588,7 @@ class TestJavascriptAssetsBundle(FileTouchable): rtl_bundle1 = self._get_asset(self.cssbundle_name, env=self.env(context={'lang': 'ar_SY'})) rtl_bundle1.css() rtl_files1 = rtl_bundle1.files - rtl_version1 = rtl_bundle1.version + rtl_version1 = rtl_bundle1.get_version('css') rtl_ira1 = self._any_ira_for_bundle('min.css', lang='ar_SY') self.assertNotEqual(rtl_files0, rtl_files1) @@ -582,8 +606,8 @@ class TestJavascriptAssetsBundle(FileTouchable): def test_19_css_in_debug_assets(self): """ Checks that a bundle rendered in debug mode(assets) with right to left language direction stores css files in assets bundle. """ - debug_bundle = self._get_asset(self.cssbundle_name, env=self.env(context={'lang': 'ar_SY'})) - nodes = debug_bundle.to_node(debug='assets') + debug_bundle = self._get_asset(self.cssbundle_name, env=self.env(context={'lang': 'ar_SY'}), debug='assets') + nodes = debug_bundle.to_node() content = self._node_to_list(nodes) # there should be an css assets bundle in /debug/rtl if user's lang direction is rtl and debug=assets @@ -751,24 +775,32 @@ class TestAssetsBundleWithIRAMock(FileTouchable): self.patch(AssetsBundle, '_unlink_attachments', unlink) def _get_asset(self): - files, _ = self.env['ir.qweb']._get_asset_content(self.stylebundle_name) - return AssetsBundle(self.stylebundle_name, files, env=self.env) + with patch.object(type(self.env['ir.asset']), '_get_installed_addons_list', Mock(return_value=self.installed_modules)): + files, _ = self.env['ir.qweb']._get_asset_content(self.stylebundle_name) + return AssetsBundle(self.stylebundle_name, files, env=self.env, debug='assets') - def _bundle(self, asset, should_create, should_unlink): + def _bundle(self, asset, should_create, should_unlink, reason=''): self.counter.clear() - asset.to_node(debug='assets') - self.assertEqual(self.counter['create'], 2 if should_create else 0) - self.assertEqual(self.counter['unlink'], 2 if should_unlink else 0) + asset.to_node() + if should_create: + self.assertEqual(self.counter['create'], 2, f'An attachment should have been created {reason}') + else: + self.assertEqual(self.counter['create'], 0, f'No attachment should have been created {reason}') + + if should_unlink: + self.assertEqual(self.counter['unlink'], 2, f'An attachment should have been unlink {reason}') + else: + self.assertEqual(self.counter['unlink'], 0, f'No attachment should have been unlink {reason}') def test_01_debug_mode_assets(self): """ Checks that the ir.attachments records created for compiled assets in debug mode are correctly invalidated. """ # Compile for the first time - self._bundle(self._get_asset(), True, False) + self._bundle(self._get_asset(), True, False, '(First access)') # Compile a second time, without changes - self._bundle(self._get_asset(), False, False) + self._bundle(self._get_asset(), False, False, '(Second access, no change)') # Touch the file and compile a third time path = get_resource_path('test_assetsbundle', 'static', 'src', 'scss', 'test_file1.scss') @@ -1065,7 +1097,7 @@ class TestAssetsManifest(AddonManifestPatched): 'name': 'test_jsfile4', 'bundle': 'test_assetsbundle.manifest2', 'directive': 'remove', - 'path': 'test_assetsbundle/static/src/**/*', + 'path': 'test_assetsbundle/static/src/*/**', }) self.env['ir.qweb']._render(view.id) attach = self.env['ir.attachment'].search([('name', 'ilike', 'test_assetsbundle.manifest2.js')], order='create_date DESC', limit=1) @@ -1758,10 +1790,10 @@ class TestAssetsManifest(AddonManifestPatched): view = self.make_asset_view('test_assetsbundle.irassetsec') self.env['ir.qweb']._render(view.id) attach = self.env['ir.attachment'].search([('name', 'ilike', 'test_assetsbundle.irassetsec')], order='create_date DESC', limit=1) - self.assertFalse(attach.exists()) + self.assertIn(b"Could not get content for /test_assetsbundle/../../tests/dummy.js", attach.exists().raw) @mute_logger('odoo.addons.base.models.ir_asset') - def test_32(self): + def test_32_a_relative_path_in_addon(self): path_to_dummy = '../../tests/dummy.xml' me = pathlib.Path(__file__).parent.absolute() file_path = me.joinpath("..", path_to_dummy) # assuming me = test_assetsbundle/tests @@ -1773,8 +1805,25 @@ class TestAssetsManifest(AddonManifestPatched): 'path': '/test_assetsbundle/%s' % path_to_dummy, }) - files = self.env['ir.asset']._get_asset_paths('test_assetsbundle.irassetsec', addons=list(self.installed_modules)) - self.assertFalse(files) + files = self.env['ir.asset']._get_asset_paths('test_assetsbundle.irassetsec') + self.assertEqual(files, [('/test_assetsbundle/../../tests/dummy.xml', None, 'test_assetsbundle.irassetsec', None)]) + # TODO, validate this behaviour + # the idea is that if the second element is False (not None) it will be added to the assetbundle, but considered in any case as an attachment url) + + @mute_logger('odoo.addons.base.models.ir_asset') + def test_32_b_relative_path_outsied_addon(self): + path_to_dummy = '../../tests/dummy.xml' + me = pathlib.Path(__file__).parent.absolute() + file_path = me.joinpath("..", path_to_dummy) # assuming me = test_assetsbundle/tests + self.assertTrue(os.path.isfile(file_path)) + + self.env['ir.asset'].create({ + 'name': '1', + 'bundle': 'test_assetsbundle.irassetsec', + 'path': '%s' % path_to_dummy, + }) + files = self.env['ir.asset']._get_asset_paths('test_assetsbundle.irassetsec') + self.assertEqual(files, [('../../tests/dummy.xml', None, 'test_assetsbundle.irassetsec', None)]) def test_33(self): self.manifests['notinstalled_module'] = { @@ -1822,8 +1871,8 @@ class TestAssetsManifest(AddonManifestPatched): 'bundle': 'test_assetsbundle.irassetsec', 'path': '/test_assetsbundle/data/ir_asset.xml', }) - files = self.env['ir.asset']._get_asset_paths('test_assetsbundle.irassetsec', addons=list(self.installed_modules)) - self.assertFalse(files) + files = self.env['ir.asset']._get_asset_paths('test_assetsbundle.irassetsec') + self.assertEqual(files, [('/test_assetsbundle/data/ir_asset.xml', None, 'test_assetsbundle.irassetsec', None)]) def test_36(self): self.env['ir.asset'].create({ @@ -1831,9 +1880,17 @@ class TestAssetsManifest(AddonManifestPatched): 'bundle': 'test_assetsbundle.irassetsec', 'path': '/test_assetsbundle/static/accessible.xml', }) - files = self.env['ir.asset']._get_asset_paths('test_assetsbundle.irassetsec', addons=list(self.installed_modules)) - self.assertEqual(len(files), 1) - self.assertTrue('test_assetsbundle/static/accessible.xml' in files[0][0]) + files = self.env['ir.asset']._get_asset_paths('test_assetsbundle.irassetsec') + modified = files[0][3] + + base_path = __file__.replace('/tests/test_assetsbundle.py', '') + + self.assertEqual(files, [( + '/test_assetsbundle/static/accessible.xml', + f'{base_path}/static/accessible.xml', + 'test_assetsbundle.irassetsec', + modified + )]) def test_37_path_can_be_an_attachment(self): scss_code = base64.b64encode(b""" diff --git a/odoo/addons/test_lint/tests/test_pylint.py b/odoo/addons/test_lint/tests/test_pylint.py index b436b24e1f6..04f00016e20 100644 --- a/odoo/addons/test_lint/tests/test_pylint.py +++ b/odoo/addons/test_lint/tests/test_pylint.py @@ -42,7 +42,7 @@ class TestPyLint(TransactionCase): 'csv', 'urllib', 'cgi', - ] + list(tools.SUPPORTED_DEBUGGER) + ] + list(tools.constants.SUPPORTED_DEBUGGER) def _skip_test(self, reason): _logger.warning(reason) diff --git a/odoo/tools/__init__.py b/odoo/tools/__init__.py index f08fc9bf412..21587bffdd2 100644 --- a/odoo/tools/__init__.py +++ b/odoo/tools/__init__.py @@ -1,9 +1,9 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -SUPPORTED_DEBUGGER = {'pdb', 'ipdb', 'wdb', 'pudb'} from . import _monkeypatches from . import appdirs from . import cloc +from . import constants from . import pdf from . import pycompat from . import win32 diff --git a/odoo/tools/constants.py b/odoo/tools/constants.py new file mode 100644 index 00000000000..89d246a28f6 --- /dev/null +++ b/odoo/tools/constants.py @@ -0,0 +1,10 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +SCRIPT_EXTENSIONS = ('js',) +STYLE_EXTENSIONS = ('css', 'scss', 'sass', 'less') +TEMPLATE_EXTENSIONS = ('xml',) +ASSET_EXTENSIONS = SCRIPT_EXTENSIONS + STYLE_EXTENSIONS + TEMPLATE_EXTENSIONS + +SUPPORTED_DEBUGGER = {'pdb', 'ipdb', 'wdb', 'pudb'} +EXTERNAL_ASSET = object()