From ee228cd83795a8356d5f87ec195c81db05cb0476 Mon Sep 17 00:00:00 2001 From: Simon Lejeune Date: Tue, 26 Jul 2016 13:28:00 +0200 Subject: [PATCH] [FIX] ir_qweb: issue if multiple IRA are created for a bundle If multiple transactions start knowing that they should generate the bundle, each one will run lessc and create a ir.attachment record referencing the path on the filestore. While the file is not duplicated on the filestore (as it is stored according to its hash), the final result of `t-call-assets` will source multiple time the same file. This patch fixes this issue by adapting get_attachment to group its results by file name and only select the one with the max id (latest generated). We also remove the `inc` argument as it is not used. --- openerp/addons/base/ir/ir_qweb.py | 22 +++++++++++++++---- .../tests/test_assetsbundle.py | 18 +++++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/openerp/addons/base/ir/ir_qweb.py b/openerp/addons/base/ir/ir_qweb.py index dab16a7f561..cfd22da7290 100644 --- a/openerp/addons/base/ir/ir_qweb.py +++ b/openerp/addons/base/ir/ir_qweb.py @@ -1250,10 +1250,24 @@ class AssetsBundle(object): ] return ira.sudo().search(domain).unlink() - def get_attachments(self, type, inc=None): - ira = self.env['ir.attachment'] - domain = [('url', '=like', '/web/content/%%-%s/%s%s.%s' % (self.version, self.xmlid, ('%%' if inc is None else '.%s' % inc), type))] - return ira.sudo().search(domain, order='name asc') + def get_attachments(self, type): + """ Return the ir.attachment records for a given bundle. This method takes care of mitigating + an issue happening when parallel transactions generate the same bundle: while the file is not + duplicated on the filestore (as it is stored according to its hash), there are multiple + ir.attachment records referencing the same version of a bundle. As we don't want to source + multiple time the same bundle in our `to_html` function, we group our ir.attachment records + by file name and only return the one with the max id for each group. + """ + url_pattern = '/web/content/%-{0}/{1}{2}.{3}'.format(self.version, self.xmlid, '.%' if type == 'css' else '', type) + self.env.cr.execute(""" + SELECT max(id) + FROM ir_attachment + WHERE url like %s + GROUP BY datas_fname + ORDER BY datas_fname + """, [url_pattern]) + attachment_ids = [r[0] for r in self.env.cr.fetchall()] + return self.env['ir.attachment'].sudo().browse(attachment_ids) def save_attachment(self, type, content, inc=None): ira = self.env['ir.attachment'] diff --git a/openerp/addons/test_assetsbundle/tests/test_assetsbundle.py b/openerp/addons/test_assetsbundle/tests/test_assetsbundle.py index 0067f006607..b078d16023e 100644 --- a/openerp/addons/test_assetsbundle/tests/test_assetsbundle.py +++ b/openerp/addons/test_assetsbundle/tests/test_assetsbundle.py @@ -276,6 +276,24 @@ class TestJavascriptAssetsBundle(TransactionCase): self.assertTrue(stylesheets[1].url.endswith('.1.css')) self.assertTrue(stylesheets[2].url.endswith('.2.css')) + def test_14_duplicated_css_assets(self): + """ Checks that if the bundle's ir.attachment record is duplicated, the bundle is only sourced once. This could + happen if multiple transactions try to render the bundle simultaneously. + """ + bundle0 = AssetsBundle(self.cssbundle_xmlid, env=self.env) + bundle0.css() + self.assertEquals(len(self._any_ira_for_bundle('css')), 1) + + # duplicate the asset bundle + ira0 = self.registry['ir.attachment'].browse(self.cr, self.uid, self._any_ira_for_bundle('css')[0]) + ira1 = ira0.copy() + self.assertEquals(len(self._any_ira_for_bundle('css')), 2) + self.assertEquals(ira0.store_fname, ira1.store_fname) + + # the ir.attachment records should be deduplicated in the bundle's content + content = bundle0.to_html() + self.assertEqual(content.count('test_assetsbundle.bundle2.0.css'), 1) + class TestAssetsBundleInBrowser(HttpCase): def test_01_js_interpretation(self):