[FIX] core: fix non-attachment binary fields for web_save

When bin_size=True is in the context, a computed non-attachment binary
is incorrectly saved to the database.  The row is actually updated with
the size of the binary instead of the value itself.

This commit fixes the problem by avoiding setting the cache with the
bin_size value as dirty.

Moreover, the binary size is computed with `pg_size_pretty` for
non-attachment binary fields.  Also, method compute_value() calls
b64decode() on the value that was previously encoded in base64 by
_compute_datas().  But _compute_datas() is specific to attachments, and
is not used in this case.  Thus b64decode() doesn't make sense.

These 3 bugs are now covered by testing web_save(), where cache
consistency is required. It was first reported for this method.

Closes #156673

closes odoo/odoo#160708

Signed-off-by: Raphael Collet <rco@odoo.com>
Co-authored-by: Renaud Thiry <reth@odoo.com>

odoo/addons/test_new_api/models/test_new_api.py

@@ -941,7 +941,10 @@ class BinarySvg(models.Model):
     name = fields.Char(required=True)
     image_attachment = fields.Binary(attachment=True)
     image_wo_attachment = fields.Binary(attachment=False)
-
+    image_wo_attachment_related = fields.Binary(
+        "image wo attachment", related="image_wo_attachment",
+        store=True, attachment=False,
+    )
 
 class MonetaryBase(models.Model):
     _name = 'test_new_api.monetary_base'

odoo/addons/test_new_api/tests/test_web_save.py

@@ -1,6 +1,8 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 from odoo.tests.common import TransactionCase
 
+from odoo.addons.base.tests.test_mimetypes import SVG, JPG
+
 
 class TestWebSave(TransactionCase):
 
@@ -36,3 +38,39 @@ class TestWebSave(TransactionCase):
         # Modify an existing record, with unity specification
         result = person.web_save({'name': 'lpe'}, {'display_name': {}})
         self.assertEqual(result, [{'id': person.id, 'display_name': 'lpe'}])
+
+    def test_web_save_computed_stored_binary(self):
+        [result] = self.env['test_new_api.binary_svg'].web_save(
+            {'name': 'test', 'image_wo_attachment': SVG},
+            {'image_wo_attachment': {}, 'image_wo_attachment_related': {}},
+        )
+        self.assertEqual(result['image_wo_attachment'], '400 bytes')  # From PostgreSQL
+        self.assertEqual(result['image_wo_attachment_related'], b'400.00 bytes')  # From human_size
+
+        # check cache values
+        record = self.env['test_new_api.binary_svg'].browse(result['id'])
+        self.assertEqual(record.image_wo_attachment, SVG)
+        self.assertEqual(record.image_wo_attachment, record.image_wo_attachment_related)
+
+        # check database values
+        self.env.invalidate_all()
+        self.assertEqual(record.image_wo_attachment, SVG)
+        self.assertEqual(record.image_wo_attachment, record.image_wo_attachment_related)
+
+        # check web_save() on existing record
+        self.env.invalidate_all()
+        [result] = record.web_save(
+            {'image_wo_attachment': JPG},
+            {'image_wo_attachment': {}, 'image_wo_attachment_related': {}},
+        )
+        self.assertEqual(result['image_wo_attachment'], '727 bytes')  # From PostgreSQL
+        self.assertEqual(result['image_wo_attachment_related'], b'727.00 bytes')  # From human_size
+
+        # check cache values
+        self.assertEqual(record.image_wo_attachment, JPG.encode())
+        self.assertEqual(record.image_wo_attachment, record.image_wo_attachment_related)
+
+        # check database values
+        self.env.invalidate_all()
+        self.assertEqual(record.image_wo_attachment, JPG.encode())
+        self.assertEqual(record.image_wo_attachment, record.image_wo_attachment_related)

odoo/fields.py

@@ -2406,18 +2406,18 @@ class Binary(Field):
             for record_no_bin_size, record in zip(records_no_bin_size, records):
                 try:
                     value = cache.get(record_no_bin_size, self)
-                    try:
-                        value = base64.b64decode(value)
-                    except (TypeError, binascii.Error):
-                        pass
+                    # don't decode non-attachments to be consistent with pg_size_pretty
+                    if not (self.store and self.column_type):
+                        with contextlib.suppress(TypeError, binascii.Error):
+                            value = base64.b64decode(value)
                     try:
                         if isinstance(value, (bytes, _BINARY)):
                             value = human_size(len(value))
                     except (TypeError):
                         pass
                     cache_value = self.convert_to_cache(value, record)
-                    dirty = self.column_type and self.store and any(records._ids)
-                    cache.set(record, self, cache_value, dirty=dirty)
+                    # the dirty flag is independent from this assignment
+                    cache.set(record, self, cache_value, check_dirty=False)
                 except CacheMiss:
                     pass
         else: