From 76adca8ec960c324d53f8f4c9bf8dfe4e0f9297e Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Thu, 3 Aug 2023 15:24:25 +0000 Subject: [PATCH] [FIX] website: clear cache on ir.asset archive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since #121376 a clear_cache was removed when unlinking an attachment This clear_cache was not useful when restarting a server with new sources, an other operation changing the content of an assets should invalidate the cache manually. This is the case of ir.asset CUD operations. Unfortunately, a manual update was left missing in website, when archiving ir.assets used for snippets. This was discovered on runbot, with two workers, when the first workers generates assets before the cron, and the second one after the cron. The second worker unlinks attachments creating an inconsistency in the cache of the first one. This problem can be solved quickly by invalidating the assets cache in the cron manually but this will be done in all cases. The proposed solution will check the ir.asset that should change and only clear the cache if the state changed. Regarding performances, this should actually be a slight improvement in query count since at the cost of one more select to prefetch the record we can avoid multiple update, one per snippet. In most case no update at all should be done, at most 2 can be done (one for archive, one for unarchive). Example with some assets to unarchive: Before: TOTAL ENTRIES: 277 SELECT: 158 (~0.16591858863830566s) UNKWOW: 54 (~0.0202481746673584s) UPDATE: 65 (~0.03202557563781738s) After: TOTAL ENTRIES: 214 SELECT: 159 (~0.1663439826965332s) UNKWOW: 54 (~0.029229164123535156s) UPDATE: 1 (~0.0002865791320800781s) If the number of query is lowered, the python processing is slightly higher. Locally the test time is similar, slightly longer since an additional call to _disable_unused_snippets_assets was added to check the cache invalidation closes odoo/odoo#130973 X-original-commit: 3aeab51ff4379e6d76172614fd39f09d6c449ac6 Signed-off-by: Xavier Dollé (xdo) --- addons/website/models/website.py | 64 ++++++------------- .../test_disable_unused_snippets_assets.py | 17 ++++- odoo/addons/base/models/ir_asset.py | 3 +- 3 files changed, 38 insertions(+), 46 deletions(-) diff --git a/addons/website/models/website.py b/addons/website/models/website.py index 7c3755d2cc3..1bb3663d7fc 100644 --- a/addons/website/models/website.py +++ b/addons/website/models/website.py @@ -1423,34 +1423,6 @@ class Website(models.Model): html_fields.append((model, table, name, translate)) return html_fields - def _get_snippets_assets(self): - """Returns every parent snippet asset from the database, filtering out - their potential overrides defined in other modules. As they share the same - snippet_id, asset_version and asset_type, it is possible to do that using - Postgres' DISTINCT ON and ordering by asset_id, as overriden assets will be - created later than their parents. - The assets are returned in the form of a list of tuples : - [(snippet_module, snippet_id, asset_version, asset_type, asset_id)] - """ - self.env.cr.execute(r""" - SELECT DISTINCT ON (snippet_id, asset_version, asset_type) - regexp_matches[1] AS snippet_module, - regexp_matches[2] AS snippet_id, - regexp_matches[3] AS asset_version, - CASE - WHEN regexp_matches[4]='scss' THEN 'css' - ELSE regexp_matches[4] - END AS asset_type, - id AS asset_id - FROM ( - SELECT REGEXP_MATCHES(PATH, '(\w*)\/.*\/snippets\/(\w*)\/(\d{3})\.(js|scss)'), - id - FROM ir_asset - ) AS regexp - ORDER BY snippet_id, asset_version, asset_type, asset_id; - """) - return self.env.cr.fetchall() - def _is_snippet_used(self, snippet_module, snippet_id, asset_version, asset_type, html_fields_attributes): snippet_occurences = [] # Check snippet template definition to avoid disabling its related assets. @@ -1488,22 +1460,28 @@ class Website(models.Model): return False def _disable_unused_snippets_assets(self): - snippets_assets = self._get_snippets_assets() + snippet_assets = self.env['ir.asset'].with_context(active_test=False).search_fetch( + [('path', 'like', '/static%/snippets/')], + ['active', 'path'], order='id') + snippet_re = re.compile(r'(\w*)\/.*\/snippets\/(\w*)\/(\d{3})(?:_\w*)?\.(js|scss)') + # regex will match /module/static/[.../]/snippets/snippet_id/XXX[_variable].asset_type + # _variable is not kept since only module, snippet_id, asset_version (XXX), asset_type are relevant html_fields_attributes = self._get_html_fields_attributes() - - for snippet_module, snippet_id, asset_version, asset_type, _asset_id in snippets_assets: - is_snippet_used = self._is_snippet_used(snippet_module, snippet_id, asset_version, asset_type, html_fields_attributes) - - # The regex catches XXX.scss, XXX.js and XXX_variables.scss - assets_regex = f'{snippet_id}/{asset_version}.+{asset_type}' - - # The query will also set to active or inactive assets overrides, as they - # share the same snippet_id, asset_version and filename_type as their parents - self.env.cr.execute(""" - UPDATE ir_asset - SET active = %(active)s - WHERE path ~ %(assets_regex)s - """, {"active": is_snippet_used, "assets_regex": assets_regex}) + snippet_used = {} + for snippet_asset in snippet_assets: + match = snippet_re.match(snippet_asset.path) + if not match: + continue + (snippet_module, snippet_id, asset_version, asset_type) = match.groups() + if asset_type == 'scss': + asset_type = 'css' + key = (snippet_id, asset_version, asset_type) # module is not relevant, we want the first one in the asset id order to filter module extension + if key not in snippet_used: + snippet_used[key] = self._is_snippet_used(snippet_module, snippet_id, asset_version, asset_type, html_fields_attributes) + is_snippet_used = snippet_used[key] + if is_snippet_used != snippet_asset.active: + snippet_asset.active = is_snippet_used + self.env['ir.asset'].flush_model() def _search_build_domain(self, domain, search, fields, extra=None): """ diff --git a/addons/website/tests/test_disable_unused_snippets_assets.py b/addons/website/tests/test_disable_unused_snippets_assets.py index 87a9d0f2f1f..148ebe7d10e 100644 --- a/addons/website/tests/test_disable_unused_snippets_assets.py +++ b/addons/website/tests/test_disable_unused_snippets_assets.py @@ -2,7 +2,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from odoo.tests import TransactionCase, tagged - +from unittest.mock import patch @tagged('post_install', '-at_install') class TestDisableSnippetsAssets(TransactionCase): @@ -68,7 +68,20 @@ class TestDisableSnippetsAssets(TransactionCase): 'mega_menu_content': MEGA_MENU_OUTDATED, }) self.mega_menu.flush_recordset() - self.Website._disable_unused_snippets_assets() + cache_clears = [] + + init_clear_cache = self.env.registry.clear_cache + def patched_clear_cache(*cache_names): + for cache_name in cache_names: + cache_clears.append(cache_name) + init_clear_cache(*cache_names) + + with patch.object(self.env.registry, 'clear_cache', patched_clear_cache): + self.Website._disable_unused_snippets_assets() + self.assertIn('assets', cache_clears, 'Assets cache should have been invalidated when updating ir_assets') + cache_clears.clear() + self.Website._disable_unused_snippets_assets() + self.assertNotIn('assets', cache_clears, 'No update on ir_assets expected, no invalidation should be triggered') s_website_form_000_scss = self._get_snippet_asset('s_website_form', '000', 'scss') s_website_form_001_scss = self._get_snippet_asset('s_website_form', '001', 'scss') diff --git a/odoo/addons/base/models/ir_asset.py b/odoo/addons/base/models/ir_asset.py index d9b8d9e9a02..bc363e859b7 100644 --- a/odoo/addons/base/models/ir_asset.py +++ b/odoo/addons/base/models/ir_asset.py @@ -68,7 +68,8 @@ class IrAsset(models.Model): return super().create(vals_list) def write(self, values): - self.env.registry.clear_cache('assets') + if self: + self.env.registry.clear_cache('assets') return super().write(values) def unlink(self):