[IMP] fs_attachment: simplify domain for attachment search by skipping res_field checks

Author: CRogos

fs_attachment/models/fs_storage.py

@@ -310,19 +310,14 @@ def recompute_urls(self) -> None:
         in staging are done in a different directory and will  not impact the
         production.
         """
-        # The weird "res_field = False OR res_field != False" domain
-        # is required! It's because of an override of _search in ir.attachment
-        # which adds ('res_field', '=', False) when the domain does not
-        # contain 'res_field'.
-        # https://github.com/odoo/odoo/blob/9032617120138848c63b3cfa5d1913c5e5ad76db/
-        # odoo/addons/base/ir/ir_attachment.py#L344-L347
         domain = [
             ("fs_storage_id", "in", self.ids),
-            "|",
-            ("res_field", "=", False),
-            ("res_field", "!=", False),
         ]
-        attachments = self.env["ir.attachment"].search(domain)
+        attachments = (
+            self.env["ir.attachment"]
+            .with_context(skip_res_field_check=True)
+            .search(domain)
+        )
         attachments._compute_fs_url()
         attachments._compute_fs_url_path()
 

fs_attachment/models/ir_attachment.py

@@ -767,19 +767,16 @@ def force_storage_to_db_for_special_fields(
                 normalize_domain(
                     [
                         ("store_fname", "=like", f"{storage}://%"),
-                        # for res_field, see comment in
-                        # _force_storage_to_object_storage
-                        "|",
-                        ("res_field", "=", False),
-                        ("res_field", "!=", False),
                     ]
                 ),
                 normalize_domain(self._store_in_db_instead_of_object_storage_domain()),
             )
         )
 
         with self._do_in_new_env(new_cr=new_cr) as new_env:
-            model_env = new_env["ir.attachment"].with_context(prefetch_fields=False)
+            model_env = new_env["ir.attachment"].with_context(
+                prefetch_fields=False, skip_res_field_check=True
+            )
             attachment_ids = model_env.search(domain).ids
             if not attachment_ids:
                 return
@@ -819,25 +816,16 @@ def _force_storage_to_object_storage(self, new_cr=False):
         storage = self.env.context.get("storage_location") or self._storage()
         if self._is_storage_disabled(storage):
             return
-        # The weird "res_field = False OR res_field != False" domain
-        # is required! It's because of an override of _search in ir.attachment
-        # which adds ('res_field', '=', False) when the domain does not
-        # contain 'res_field'.
-        # https://github.com/odoo/odoo/blob/9032617120138848c63b3cfa5d1913c5e5ad76db/
-        # odoo/addons/base/ir/ir_attachment.py#L344-L347
         domain = [
             "!",
             ("store_fname", "=like", f"{storage}://%"),
-            "|",
-            ("res_field", "=", False),
-            ("res_field", "!=", False),
         ]
         # We do a copy of the environment so we can workaround the cache issue
         # below. We do not create a new cursor by default because it causes
         # serialization issues due to concurrent updates on attachments during
         # the installation
         with self._do_in_new_env(new_cr=new_cr) as new_env:
-            model_env = new_env["ir.attachment"]
+            model_env = new_env["ir.attachment"].with_context(skip_res_field_check=True)
             ids = model_env.search(domain).ids
             files_to_clean = []
             for attachment_id in ids:

fs_attachment/tests/test_fs_attachment.py

@@ -337,24 +337,44 @@ def test_force_storage_to_db(self):
         attachment = self.ir_attachment_model.create(
             {"name": "test.txt", "raw": b"content"}
         )
+        attachment_res_field = self.ir_attachment_model.create(
+            {
+                "name": "field.txt",
+                "raw": "Attachment linked to a field",
+                "res_model": "res.partner",
+                "res_field": "name",
+            }
+        )
         self.env.flush_all()
         self.assertTrue(attachment.store_fname)
         self.assertFalse(attachment.db_datas)
         store_fname = attachment.store_fname
+        store_fname_res_field = attachment_res_field.store_fname
         # we change the rules to force the storage in db for text/plain
         self.temp_backend.force_db_for_default_attachment_rules = '{"text/plain": 0}'
         attachment.force_storage_to_db_for_special_fields()
         self.assertFalse(attachment.store_fname)
         self.assertEqual(attachment.db_datas, b"content")
+        self.assertFalse(attachment_res_field.store_fname)
+        self.assertEqual(attachment_res_field.db_datas, b"Attachment linked to a field")
         # we check that the file is marked for GC
         gc_files = self.gc_file_model.search([]).mapped("store_fname")
         self.assertIn(store_fname, gc_files)
+        self.assertIn(store_fname_res_field, gc_files)
 
     @mute_logger("odoo.addons.fs_attachment.models.ir_attachment")
     def test_force_storage_to_fs(self):
         attachment = self.ir_attachment_model.create(
             {"name": "test.txt", "raw": b"content"}
         )
+        attachment_res_field = self.ir_attachment_model.create(
+            {
+                "name": "field.txt",
+                "raw": "Attachment linked to a field",
+                "res_model": "res.partner",
+                "res_field": "name",
+            }
+        )
         self.env.flush_all()
         fs_path = self.ir_attachment_model._filestore() + "/" + attachment.store_fname
         self.assertTrue(os.path.exists(fs_path))
@@ -371,8 +391,13 @@ def test_force_storage_to_fs(self):
             clean_fs.assert_called_once()
         # files into the filestore must be moved to our filesystem storage
         filename = f"test-{attachment.id}-0.txt"
+        filename_res_field = f"field-{attachment_res_field.id}-0.txt"
         self.assertEqual(attachment.store_fname, f"tmp_dir://{filename}")
+        self.assertEqual(
+            attachment_res_field.store_fname, f"tmp_dir://{filename_res_field}"
+        )
         self.assertIn(filename, os.listdir(self.temp_dir))
+        self.assertIn(filename_res_field, os.listdir(self.temp_dir))
 
     def test_storage_use_filename_obfuscation(self):
         self.temp_backend.base_url = "https://acsone.eu/media"