[FIX] core: fix create/write on binary fields

When invoking create() or write() with a binary field, the cache of the
field was incorrect if bin_size=True was in context.  Force context with
bin_size=False when putting a binary value in cache.  It is particularly
important to have coherent values in the cache for `web_save`.

Also, because an environment with bin_size=False won't return the same
context cache key as one with bin_size=None, it leads to have a cache
inconstistency when we write with bin_size=False.  Change Environment
method cache_key() to return the same cache key when bin_size is absent,
bin_size=None or bin_size=False.

Tests on binary fields have been updated to not rely on flush and
invalidate.  We also created specific tests for write() on binary
fields.

Part-of: odoo/odoo#160708
This commit is contained in:
Rémy Voet (ryv)
2024-04-26 17:17:52 +00:00
committed by Raphael Collet
parent a0bf434960
commit 87a1ebb338
5 changed files with 83 additions and 17 deletions
@@ -931,6 +931,7 @@ class ModelImage(models.Model):
image_512 = fields.Image("Image 512", related='image', max_width=512, max_height=512, store=True, readonly=False)
image_256 = fields.Image("Image 256", related='image', max_width=256, max_height=256, store=False, readonly=False)
image_128 = fields.Image("Image 128", max_width=128, max_height=128)
image_64 = fields.Image("Image 64", related='image', max_width=64, max_height=64, store=True, attachment=False)
class BinarySvg(models.Model):
@@ -2596,6 +2596,8 @@ class TestFields(TransactionCaseWithUserDemo):
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_512))).size, (512, 256))
# test create related no store (resize, width limited)
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_256))).size, (256, 128))
# test create related store on column (resize, width limited)
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_64))).size, (64, 32))
record.write({
'image': image_h,
@@ -2610,6 +2612,8 @@ class TestFields(TransactionCaseWithUserDemo):
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_512))).size, (256, 512))
# test write related no store (resize, height limited)
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_256))).size, (128, 256))
# test write related store on column (resize, width limited)
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_64))).size, (32, 64))
record = self.env['test_new_api.model_image'].create({
'name': 'image',
@@ -2625,6 +2629,8 @@ class TestFields(TransactionCaseWithUserDemo):
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_512))).size, (256, 512))
# test create related no store (resize, height limited)
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_256))).size, (128, 256))
# test create related store on column (resize, width limited)
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_64))).size, (32, 64))
record.write({
'image': image_w,
@@ -2639,6 +2645,8 @@ class TestFields(TransactionCaseWithUserDemo):
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_512))).size, (512, 256))
# test write related store (resize, width limited)
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_256))).size, (256, 128))
# test write related store on column (resize, width limited)
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_64))).size, (64, 32))
# test create inverse store
record = self.env['test_new_api.model_image'].create({
@@ -2649,6 +2657,7 @@ class TestFields(TransactionCaseWithUserDemo):
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_512))).size, (512, 256))
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image))).size, (4000, 2000))
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_256))).size, (256, 128))
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_64))).size, (64, 32))
# test write inverse store
record.write({
'image_512': image_h,
@@ -2657,6 +2666,7 @@ class TestFields(TransactionCaseWithUserDemo):
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_512))).size, (256, 512))
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image))).size, (2000, 4000))
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_256))).size, (128, 256))
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_64))).size, (32, 64))
# test create inverse no store
record = self.env['test_new_api.model_image'].with_context(image_no_postprocess=True).create({
@@ -2667,6 +2677,7 @@ class TestFields(TransactionCaseWithUserDemo):
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_512))).size, (512, 256))
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image))).size, (4000, 2000))
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_256))).size, (256, 128))
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_64))).size, (64, 32))
# test write inverse no store
record.write({
'image_256': image_h,
@@ -2675,6 +2686,7 @@ class TestFields(TransactionCaseWithUserDemo):
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_512))).size, (256, 512))
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image))).size, (2000, 4000))
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_256))).size, (128, 256))
self.assertEqual(Image.open(io.BytesIO(base64.b64decode(record.image_64))).size, (32, 64))
# test bin_size
record_bin_size = record.with_context(bin_size=True)
@@ -2703,18 +2715,16 @@ class TestFields(TransactionCaseWithUserDemo):
with self.assertQueryCount(0):
new_record.image = image_w
def test_95_binary_bin_size(self):
def test_95_binary_bin_size_create(self):
binary_value = base64.b64encode(b'content')
binary_size = b'7.00 bytes'
def assertBinaryValue(record, value):
for field in ('binary', 'binary_related_store', 'binary_related_no_store'):
self.assertEqual(record[field], value)
self.assertEqual(record[field], value, f'Incorrect result for {field}')
# created, flushed, and first read without context
# created and first read without context
record = self.env['test_new_api.model_binary'].create({'binary': binary_value})
self.env.flush_all()
self.env.invalidate_all()
record_no_bin_size = record.with_context(bin_size=False)
record_bin_size = record.with_context(bin_size=True)
@@ -2722,10 +2732,8 @@ class TestFields(TransactionCaseWithUserDemo):
assertBinaryValue(record_no_bin_size, binary_value)
assertBinaryValue(record_bin_size, binary_size)
# created, flushed, and first read with bin_size=False
# created and first read with bin_size=False
record_no_bin_size = self.env['test_new_api.model_binary'].with_context(bin_size=False).create({'binary': binary_value})
self.env.flush_all()
self.env.invalidate_all()
record = self.env['test_new_api.model_binary'].browse(record.id)
record_bin_size = record.with_context(bin_size=True)
@@ -2733,10 +2741,8 @@ class TestFields(TransactionCaseWithUserDemo):
assertBinaryValue(record, binary_value)
assertBinaryValue(record_bin_size, binary_size)
# created, flushed, and first read with bin_size=True
# created and first read with bin_size=True
record_bin_size = self.env['test_new_api.model_binary'].with_context(bin_size=True).create({'binary': binary_value})
self.env.flush_all()
self.env.invalidate_all()
record = self.env['test_new_api.model_binary'].browse(record.id)
record_no_bin_size = record.with_context(bin_size=False)
@@ -2744,12 +2750,11 @@ class TestFields(TransactionCaseWithUserDemo):
assertBinaryValue(record_no_bin_size, binary_value)
assertBinaryValue(record, binary_value)
# created without context and flushed with bin_size
# created without context and flushed/invalidated with bin_size=True
record = self.env['test_new_api.model_binary'].create({'binary': binary_value})
record.with_context(bin_size=True).env.invalidate_all()
record_no_bin_size = record.with_context(bin_size=False)
record_bin_size = record.with_context(bin_size=True)
self.env.flush_all()
self.env.invalidate_all()
assertBinaryValue(record, binary_value)
assertBinaryValue(record_no_bin_size, binary_value)
@@ -2757,8 +2762,6 @@ class TestFields(TransactionCaseWithUserDemo):
# check computed binary field with arbitrary Python value
record = self.env['test_new_api.model_binary'].create({})
self.env.flush_all()
self.env.invalidate_all()
record_no_bin_size = record.with_context(bin_size=False)
record_bin_size = record.with_context(bin_size=True)
@@ -2767,6 +2770,64 @@ class TestFields(TransactionCaseWithUserDemo):
self.assertEqual(record_no_bin_size.binary_computed, expected_value)
self.assertEqual(record_bin_size.binary_computed, expected_value)
def test_95_binary_bin_size_write(self):
binary_value = base64.b64encode(b'content')
binary_size = b'7.00 bytes'
def assertBinaryValue(record, value):
for field in ('binary', 'binary_related_store', 'binary_related_no_store'):
self.assertEqual(record[field], value, f'Incorrect result for {field}')
# created and written without context
record = self.env['test_new_api.model_binary'].create({})
record.write({'binary': binary_value})
record_no_bin_size = record.with_context(bin_size=False)
record_bin_size = record.with_context(bin_size=True)
assertBinaryValue(record, binary_value)
assertBinaryValue(record_no_bin_size, binary_value)
assertBinaryValue(record_bin_size, binary_size)
# created without context, written with bin_size=False
record = self.env['test_new_api.model_binary'].create({})
record.with_context(bin_size=False).write({'binary': binary_value})
record_bin_size = record.with_context(bin_size=True)
assertBinaryValue(record_no_bin_size, binary_value)
assertBinaryValue(record, binary_value)
assertBinaryValue(record_bin_size, binary_size)
# created without context, written with bin_size=True
record = self.env['test_new_api.model_binary'].create({})
record.with_context(bin_size=True).write({'binary': binary_value})
record_no_bin_size = record.with_context(bin_size=False)
assertBinaryValue(record_bin_size, binary_size)
assertBinaryValue(record_no_bin_size, binary_value)
assertBinaryValue(record, binary_value)
# created without context and flushed with bin_size=True
record = self.env['test_new_api.model_binary'].create({})
record.write({'binary': binary_value})
record.with_context(bin_size=True).env.invalidate_all()
record_no_bin_size = record.with_context(bin_size=False)
record_bin_size = record.with_context(bin_size=True)
assertBinaryValue(record, binary_value)
assertBinaryValue(record_no_bin_size, binary_value)
assertBinaryValue(record_bin_size, binary_size)
# created and written without context, flushed without bin_size
record = self.env['test_new_api.model_binary'].create({})
record.write({'binary': binary_value})
record.env.invalidate_all()
record_no_bin_size = record.with_context(bin_size=False)
record_bin_size = record.with_context(bin_size=True)
assertBinaryValue(record, binary_value)
assertBinaryValue(record_no_bin_size, binary_value)
assertBinaryValue(record_bin_size, binary_size)
def test_96_order_m2o(self):
belgium, congo = self.env['test_new_api.country'].create([
{'name': "Duchy of Brabant"},
+2
View File
@@ -815,6 +815,8 @@ class Environment(Mapping):
return get_context('lang') or None
elif key == 'active_test':
return get_context('active_test', field.context.get('active_test', True))
elif key.startswith('bin_size'):
return bool(get_context(key))
else:
val = get_context(key)
if type(val) is list:
+1
View File
@@ -2458,6 +2458,7 @@ class Binary(Field):
])
def write(self, records, value):
records = records.with_context(bin_size=False)
if not self.attachment:
super().write(records, value)
return
+2 -1
View File
@@ -4797,13 +4797,14 @@ class BaseModel(metaclass=MetaModel):
ids.extend(id_ for id_, in cr.fetchall())
# put the new records in cache, and update inverse fields, for many2one
# (using bin_size=False to put binary values in the right place)
#
# cachetoclear is an optimization to avoid modified()'s cost until other_fields are processed
cachetoclear = []
records = self.browse(ids)
inverses_update = defaultdict(list) # {(field, value): ids}
common_set_vals = set(LOG_ACCESS_COLUMNS + ['id', 'parent_path'])
for data, record in zip(data_list, records):
for data, record in zip(data_list, records.with_context(bin_size=False)):
data['record'] = record
# DLE P104: test_inherit.py, test_50_search_one2many
vals = dict({k: v for d in data['inherited'].values() for k, v in d.items()}, **data['stored'])