[19.0][FIX] fs_attachment: don't move a file shared with other attachments

fs_attachment/models/ir_attachment.py

@@ -479,30 +479,35 @@ def _enforce_meaningful_storage_filename(self) -> None:
         Keeping the same meaning and mimetype is important to also ease to provide
         a meaningful and SEO friendly URL to the file in the filesystem storage.
         """
-        renamed_attachments = {}
+        to_rename = self.browse()
         for attachment in self:
             if not self._is_file_from_a_storage(attachment.store_fname):
                 continue
             fs, storage, filename = attachment._get_fs_parts()
 
             if self.env["fs.storage"]._must_use_filename_obfuscation(storage):
                 attachment.fs_filename = filename
-                continue
+            else:
+                to_rename |= attachment
+        to_rename._set_meaningful_storage_filename()
+
+    def _set_meaningful_storage_filename(self) -> None:
+        """Give each attachment's file its meaningful name"""
+        usage = self._count_by_store_fname()
+        for attachment in self:
+            fs, storage, filename = attachment._get_fs_parts()
             new_filename = attachment._build_fs_filename()
             # we must keep the same full path as the original filename
             new_filename_with_path = os.path.join(
                 os.path.dirname(filename), new_filename
             )
 
-            if filename in renamed_attachments:
-                if renamed_attachments[filename] == new_filename_with_path:
-                    # we already renamed this file, no need to rename it again
-                    continue
-                else:
-                    fs.copy(renamed_attachments[filename], new_filename_with_path)
+            if usage[attachment.store_fname] > 1:
+                # the file stays with the other attachments stored in it
+                usage[attachment.store_fname] -= 1
+                fs.copy(filename, new_filename_with_path)
             else:
                 fs.rename(filename, new_filename_with_path)
-            renamed_attachments[filename] = new_filename_with_path
 
             attachment.fs_filename = new_filename
             # we need to update the store_fname with the new filename by
@@ -512,6 +517,19 @@ def _enforce_meaningful_storage_filename(self) -> None:
             attachment._force_write_store_fname(f"{storage}://{new_filename_with_path}")
             self._fs_mark_for_gc(attachment.store_fname)
 
+    def _count_by_store_fname(self) -> dict[str, int]:
+        """Return the number of attachments stored in each file of self"""
+        # "res_field = False OR res_field != False" to count the attachments of
+        # binary fields too, see fs.storage
+        domain = [
+            ("store_fname", "in", self.mapped("store_fname")),
+            "|",
+            ("res_field", "=", False),
+            ("res_field", "!=", False),
+        ]
+        groups = self.sudo()._read_group(domain, ["store_fname"], ["__count"])
+        return dict(groups)
+
     def _force_write_store_fname(self, store_fname):
         """Force the write of the store_fname field
 

fs_attachment/readme/newsfragments/678.bugfix

@@ -0,0 +1,6 @@
+Don't move a file that other attachments still use.
+
+With filename obfuscation off, storing or renaming an attachment renames its
+file. Attachments stored while obfuscation was on share one file per content,
+so the others lost their content. The module now copies such a file and only
+moves it when no other attachment uses it.

fs_attachment/tests/test_fs_attachment.py

@@ -502,6 +502,59 @@ def test_create_two_attachments_differnt_call(self):
         self.assertNotEqual(res[0].store_fname, res2[0].store_fname)
         self.assertEqual(res[0].raw, res2[0].raw)
 
+    def test_create_attachment_same_content_as_obfuscated_one(self):
+        self.temp_backend.use_as_default_for_attachments = True
+        self.temp_backend.use_filename_obfuscation = True
+        attachment1 = self.ir_attachment_model.create(
+            {"name": "test.txt", "raw": b"content"}
+        )
+        self.temp_backend.use_filename_obfuscation = False
+        attachment2 = self.ir_attachment_model.create(
+            {"name": "test.txt", "raw": b"content"}
+        )
+        self.assertNotEqual(attachment1.store_fname, attachment2.store_fname)
+        self.env.invalidate_all()
+        self.assertEqual(attachment1.raw, b"content")
+        self.assertEqual(attachment2.raw, b"content")
+
+    def test_create_attachment_same_content_as_obfuscated_field_one(self):
+        self.temp_backend.use_as_default_for_attachments = True
+        self.temp_backend.use_filename_obfuscation = True
+        attachment1 = self.ir_attachment_model.create(
+            {
+                "name": "test.txt",
+                "raw": b"content",
+                "res_id": self.env.user.partner_id.id,
+                "res_model": "res.partner",
+                "res_field": "image_1920",
+            }
+        )
+        self.temp_backend.use_filename_obfuscation = False
+        attachment2 = self.ir_attachment_model.create(
+            {"name": "test.txt", "raw": b"content"}
+        )
+        self.assertNotEqual(attachment1.store_fname, attachment2.store_fname)
+        self.env.invalidate_all()
+        self.assertEqual(attachment1.raw, b"content")
+        self.assertEqual(attachment2.raw, b"content")
+
+    def test_write_name_file_shared_with_obfuscated_attachment(self):
+        self.temp_backend.use_as_default_for_attachments = True
+        self.temp_backend.use_filename_obfuscation = True
+        attachment1, attachment2 = self.ir_attachment_model.create(
+            [
+                {"name": "test.txt", "raw": b"content"},
+                {"name": "test.txt", "raw": b"content"},
+            ]
+        )
+        self.assertEqual(attachment1.store_fname, attachment2.store_fname)
+        self.temp_backend.use_filename_obfuscation = False
+        attachment1.name = "test2.txt"
+        self.assertNotEqual(attachment1.store_fname, attachment2.store_fname)
+        self.env.invalidate_all()
+        self.assertEqual(attachment1.raw, b"content")
+        self.assertEqual(attachment2.raw, b"content")
+
     def test_update_png_to_svg(self):
         b64_data_png = (
             b"iVBORw0KGgoAAAANSUhEUgAAADMAAAAhCAIAAAD73QTtAAAAA3NCSVQICAjb4U/gAA"