From 4710827b2181923d6c59ae5f28938b11f0a2034c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Mart=C3=ADnez?= Date: Mon, 22 Jul 2024 12:12:16 +0200 Subject: [PATCH 1/2] [IMP+FIX] dms: Improve performance + Avoid non-existing record access error Changes done: - Improve _dms_operations() method to do nothing if the model is not one of those used in dms.storage. - Avoid access error with .browse() if we access a non-existing record (for example, deleted by database). TT50231 --- dms/models/dms_security_mixin.py | 7 +++++-- dms/models/ir_attachment.py | 20 +++++++++++++++++++- dms/models/storage.py | 6 ++++++ dms/tests/test_storage_attachment.py | 13 +++++++++++++ 4 files changed, 43 insertions(+), 3 deletions(-) diff --git a/dms/models/dms_security_mixin.py b/dms/models/dms_security_mixin.py index d982f52cc..3766708ff 100644 --- a/dms/models/dms_security_mixin.py +++ b/dms/models/dms_security_mixin.py @@ -139,8 +139,11 @@ def _get_domain_by_inheritance(self, operation): continue domains.append([("res_model", "=", model._name), ("res_id", "=", False)]) # Check record access in batch too - group_ids = [i for i in group["res_id"] if i] # Hack to remove None res_id - related_ok = model.browse(group_ids)._filter_access_rules_python(operation) + res_ids = [i for i in group["res_id"] if i] # Hack to remove None res_id + # Apply exists to skip records that do not exist. (e.g. a res.partner + # deleted by database). + model_records = model.browse(res_ids).exists() + related_ok = model_records._filter_access_rules_python(operation) if not related_ok: continue domains.append( diff --git a/dms/models/ir_attachment.py b/dms/models/ir_attachment.py index 799cc5acb..dbd4d4119 100644 --- a/dms/models/ir_attachment.py +++ b/dms/models/ir_attachment.py @@ -1,6 +1,7 @@ # Copyright 2021 Tecnativa - Víctor Martínez # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl). from odoo import api, models +from odoo.tools import ormcache class IrAttachment(models.Model): @@ -34,9 +35,26 @@ def _dms_directories_create(self): } ) + @ormcache("model") + def _dms_operations_from_model(self, model): + # Apply sudo to prevent ir.rule from being applied. + item = self.env["dms.storage"].sudo().search([("model_ids.model", "=", model)]) + return bool(item) + def _dms_operations(self): + """Perform the operation only if there is a storage with linked models. + The directory (dms.directory) linked to the record (if it does not exist) + and the file (dms.file) with the linked attachment would be created. + """ for attachment in self: - if not attachment.res_model or not attachment.res_id: + if ( + not attachment.res_model + or not attachment.res_id + or ( + attachment.res_model + and not self._dms_operations_from_model(attachment.res_model) + ) + ): continue directories = attachment._get_dms_directories( attachment.res_model, attachment.res_id diff --git a/dms/models/storage.py b/dms/models/storage.py index 41576fd74..db5d261e3 100644 --- a/dms/models/storage.py +++ b/dms/models/storage.py @@ -125,3 +125,9 @@ def _compute_count_storage_directories(self): def _compute_count_storage_files(self): for record in self: record.count_storage_files = len(record.storage_file_ids) + + def write(self, values): + res = super().write(values) + if "model_ids" in values: + self.env.registry.clear_cache() + return res diff --git a/dms/tests/test_storage_attachment.py b/dms/tests/test_storage_attachment.py index a1ab807fe..5d9465a8a 100644 --- a/dms/tests/test_storage_attachment.py +++ b/dms/tests/test_storage_attachment.py @@ -35,6 +35,19 @@ def test_storage_attachment(self): self.assertFalse(file_01.exists(), "File should not exist") self.assertFalse(directory.exists(), "Directory should not exist") + @users("dms-manager") + def test_storage_attachment_record_db_unlink(self): + self._create_attachment("demo.txt") + self.assertTrue( + self.storage.storage_file_ids.filtered(lambda x: x.name == "demo.txt") + ) + directory = self._get_partner_directory() + self.assertEqual(directory.res_model, self.partner._name) + self.assertEqual(directory.res_id, self.partner.id) + directory.res_id = -1 # Trick to reference a non-existing record + directories = self.env["dms.directory"].search([]) + self.assertNotIn(directory.id, directories.ids) + @users("dms-manager") def test_storage_attachment_misc(self): attachment = self._create_attachment("demo.txt") From 515372f3e23b16087c84e78a86d3bcdba3574228 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Mart=C3=ADnez?= Date: Tue, 20 Aug 2024 08:31:50 +0200 Subject: [PATCH 2/2] [FIX] dms: Remove log warning in dms.file UserWarning: dms.file: inconsistent 'store' for computed fields, accessing migration may recompute and update require_migration. Use distinct compute methods for stored and non-stored fields --- dms/models/dms_file.py | 1 + 1 file changed, 1 insertion(+) diff --git a/dms/models/dms_file.py b/dms/models/dms_file.py index 7dc84f639..21b698d72 100644 --- a/dms/models/dms_file.py +++ b/dms/models/dms_file.py @@ -125,6 +125,7 @@ class DMSFile(models.Model): readonly=True, prefetch=False, compute_sudo=True, + store=True, ) require_migration = fields.Boolean( compute="_compute_migration", store=True, compute_sudo=True