Skip to content

Commit

Permalink
[FIX] core: fix non-attachment binary fields for web_save
Browse files Browse the repository at this point in the history
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

Co-authored-by: Renaud Thiry <reth@odoo.com>
  • Loading branch information
ryv-odoo and reth-odoo committed Apr 29, 2024
1 parent 726cdfa commit 0e86d78
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 7 deletions.
5 changes: 4 additions & 1 deletion odoo/addons/test_new_api/models/test_new_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
38 changes: 38 additions & 0 deletions odoo/addons/test_new_api/tests/test_web_save.py
Original file line number Diff line number Diff line change
@@ -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):

Expand Down Expand Up @@ -36,3 +38,39 @@ def test_web_save_write(self):
# 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)
12 changes: 6 additions & 6 deletions odoo/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -2391,18 +2391,18 @@ def compute_value(self, records):
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:
Expand Down

0 comments on commit 0e86d78

Please sign in to comment.