Merge PR #307 into 16.0

Signed-off-by lmignon

fs_attachment/README.rst

@@ -7,7 +7,7 @@ Base Attachment Object Store
    !! This file is generated by oca-gen-addon-readme !!
    !! changes will be overwritten.                   !!
    !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
-   !! source digest: sha256:d4168f6264cd4c82abb4458d95d95b6b8aa21a8534756a166820d882224da8fc
+   !! source digest: sha256:866960d18aa5896dcc4b673c7af29c2891dfb5da3df395c39e0b03bce2f255b1
    !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 
 .. |badge1| image:: https://img.shields.io/badge/maturity-Beta-yellow.png

fs_attachment/models/ir_attachment.py

@@ -112,9 +112,14 @@ def _compute_internal_url(self) -> None:
     @api.depends("fs_filename")
     def _compute_fs_url(self) -> None:
         for rec in self:
-            rec.fs_url = None
+            new_url = None
+            actual_url = rec.fs_url or None
             if rec.fs_filename:
-                rec.fs_url = self.env["fs.storage"]._get_url_for_attachment(rec)
+                new_url = self.env["fs.storage"]._get_url_for_attachment(rec)
+            # ensure we compare value of same type and not None with False
+            new_url = new_url or None
+            if new_url != actual_url:
+                rec.fs_url = new_url
 
     @api.depends("fs_filename")
     def _compute_fs_url_path(self) -> None:

fs_attachment/readme/newsfragments/307.bugfix

@@ -0,0 +1,7 @@
+Improve performance at creation of an attachment or when the attachment is updated.
+
+Before this change, when the fs_url was computed the computed value was always
+reassigned to the fs_url attribute even if the value was the same. In a lot of
+cases the value was the same and the reassignment was not necessary. Unfortunately
+this reassignment has as side effect to mark the record as dirty and generate a
+SQL update statement at the end of the transaction.

fs_attachment/static/description/index.html

@@ -367,7 +367,7 @@ <h1 class="title">Base Attachment Object Store</h1>
 !! This file is generated by oca-gen-addon-readme !!
 !! changes will be overwritten.                   !!
 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
-!! source digest: sha256:d4168f6264cd4c82abb4458d95d95b6b8aa21a8534756a166820d882224da8fc
+!! source digest: sha256:866960d18aa5896dcc4b673c7af29c2891dfb5da3df395c39e0b03bce2f255b1
 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -->
 <p><a class="reference external image-reference" href="https://odoo-community.org/page/development-status"><img alt="Beta" src="https://img.shields.io/badge/maturity-Beta-yellow.png" /></a> <a class="reference external image-reference" href="http://www.gnu.org/licenses/agpl-3.0-standalone.html"><img alt="License: AGPL-3" src="https://img.shields.io/badge/licence-AGPL--3-blue.png" /></a> <a class="reference external image-reference" href="https://github.com/OCA/storage/tree/16.0/fs_attachment"><img alt="OCA/storage" src="https://img.shields.io/badge/github-OCA%2Fstorage-lightgray.png?logo=github" /></a> <a class="reference external image-reference" href="https://translation.odoo-community.org/projects/storage-16-0/storage-16-0-fs_attachment"><img alt="Translate me on Weblate" src="https://img.shields.io/badge/weblate-Translate%20me-F47D42.png" /></a> <a class="reference external image-reference" href="https://runboat.odoo-community.org/builds?repo=OCA/storage&amp;target_branch=16.0"><img alt="Try me on Runboat" src="https://img.shields.io/badge/runboat-Try%20me-875A7B.png" /></a></p>
 <p>In some cases, you need to store attachment in another system that the Odoo’s

fs_image/README.rst

@@ -7,7 +7,7 @@ Fs Image
    !! This file is generated by oca-gen-addon-readme !!
    !! changes will be overwritten.                   !!
    !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
-   !! source digest: sha256:117413401c2187f3807cd4ce73ae108acacc5347ac150ec1fce965b5d073ee25
+   !! source digest: sha256:8743df12cbf6e8e739aa2d6241767c1f54c7ab74c7956fb93523758dbaa19a66
    !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 
 .. |badge1| image:: https://img.shields.io/badge/maturity-Alpha-red.png

fs_image/fields.py

@@ -23,7 +23,13 @@ def __init__(
         alt_text: str = None,
     ) -> None:
         super().__init__(attachment, name, value)
-        self.alt_text = alt_text
+        if self._attachment and alt_text is not None:
+            raise ValueError(
+                "FSImageValue cannot be initialized with an attachment and an"
+                " alt_text at the same time. When initializing with an attachment,"
+                " you can't pass any other argument."
+            )
+        self._alt_text = alt_text
 
     @property
     def alt_text(self) -> str:
@@ -41,17 +47,12 @@ def alt_text(self, value: str) -> None:
     def from_fs_file_value(cls, fs_file_value: FSFileValue) -> "FSImageValue":
         if isinstance(fs_file_value, FSImageValue):
             return fs_file_value
-        alt_text = (
-            fs_file_value.attachment.alt_text if fs_file_value.attachment else None
-        )
-        alt_text = alt_text or None
         return cls(
             attachment=fs_file_value.attachment,
             name=fs_file_value.name if not fs_file_value.attachment else None,
             value=fs_file_value._buffer
             if not fs_file_value.attachment
             else fs_file_value._buffer,
-            alt_text=alt_text,
         )
 
     def image_process(
@@ -167,7 +168,8 @@ def _create_attachment(self, record, cache_value):
         attachment = super()._create_attachment(record, cache_value)
         # odoo filter out additional fields in create method on ir.attachment
         # so we need to write the alt_text after the creation
-        attachment.alt_text = cache_value.alt_text
+        if cache_value.alt_text:
+            attachment.alt_text = cache_value.alt_text
         return attachment
 
     def _convert_attachment_to_cache(self, attachment: IrAttachment) -> FSImageValue:

fs_image/readme/newsfragments/307.bugfix

@@ -0,0 +1,10 @@
+Avoid to generate an SQL update query when an image field is read.
+
+Fix a bug in the initialization of the image field value object when the field
+is read. Before this fix, every time the value object was initialized with
+an attachment, an assignment of the alt text was done into the constructor.
+This assignment triggered the mark of the field as modified and an SQL update
+query was generated at the end of the request. The alt text in the constructor
+of the FSImageValue class must only be used when the class is initialized without
+an attachment. We now check if an attachment and an alt text are provided at
+the same time and throw an exception if this is the case.

fs_image/static/description/index.html

@@ -367,7 +367,7 @@ <h1 class="title">Fs Image</h1>
 !! This file is generated by oca-gen-addon-readme !!
 !! changes will be overwritten.                   !!
 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
-!! source digest: sha256:117413401c2187f3807cd4ce73ae108acacc5347ac150ec1fce965b5d073ee25
+!! source digest: sha256:8743df12cbf6e8e739aa2d6241767c1f54c7ab74c7956fb93523758dbaa19a66
 !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! -->
 <p><a class="reference external image-reference" href="https://odoo-community.org/page/development-status"><img alt="Alpha" src="https://img.shields.io/badge/maturity-Alpha-red.png" /></a> <a class="reference external image-reference" href="http://www.gnu.org/licenses/agpl-3.0-standalone.html"><img alt="License: AGPL-3" src="https://img.shields.io/badge/licence-AGPL--3-blue.png" /></a> <a class="reference external image-reference" href="https://github.com/OCA/storage/tree/16.0/fs_image"><img alt="OCA/storage" src="https://img.shields.io/badge/github-OCA%2Fstorage-lightgray.png?logo=github" /></a> <a class="reference external image-reference" href="https://translation.odoo-community.org/projects/storage-16-0/storage-16-0-fs_image"><img alt="Translate me on Weblate" src="https://img.shields.io/badge/weblate-Translate%20me-F47D42.png" /></a> <a class="reference external image-reference" href="https://runboat.odoo-community.org/builds?repo=OCA/storage&amp;target_branch=16.0"><img alt="Try me on Runboat" src="https://img.shields.io/badge/runboat-Try%20me-875A7B.png" /></a></p>
 <p>This addon defines a new field <strong>FSImage</strong> to use in your models. It is a

fs_image/tests/test_fs_image.py

@@ -9,7 +9,7 @@
 from PIL import Image
 
 from odoo.exceptions import UserError
-from odoo.tests.common import TransactionCase
+from odoo.tests.common import TransactionCase, users, warmup
 
 from odoo.addons.fs_storage.models.fs_storage import FSStorage
 
@@ -220,3 +220,20 @@ def test_write_alt_text_on_empty_with_dict(self):
         instance = self.env["test.image.model"].create({})
         with self.assertRaisesRegex(UserError, "Cannot set alt_text on empty image"):
             instance.write({"fs_image": {"alt_text": "test"}})
+
+    @users("__system__")
+    @warmup
+    def test_generated_sql_commands(self):
+        # The following tests will never fail, but they will output a warning
+        # if the number of SQL queries changes into the logs. They
+        # are to help us keep track of the number of SQL queries generated
+        # by the module.
+        with self.assertQueryCount(__system__=3):
+            instance = self.env["test.image.model"].create(
+                {"fs_image": FSImageValue(name=self.filename, value=self.image_w)}
+            )
+
+        instance.invalidate_recordset()
+        with self.assertQueryCount(__system__=1):
+            self.assertEqual(instance.fs_image.getvalue(), self.image_w)
+            self.env.flush_all()