pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] core: automatically flush upon invalidation for cache consistency

Browse Source 
Because method _read() no longer updates existing values in memory,
those values in memory must be consistent with the database.

On the other hand, if pending updates are not performed on the database
before fetching values, it means that the corresponding database values
cannot be put in cache.  This implies that one cannot empty the cache
without flushing the corresponding fields.

In order to avoid mistakes, flush automatically before invalidating the
cache.  This makes the invalidation methods safe by default, and avoids
cargo-culting which would systematically associate invalidation to
flushing, which may eventually be less performant.

Part-of: odoo/odoo#66938
This commit is contained in:
Raphael Collet
2022-07-05 11:35:00 +02:00
parent a91cb08c5d
commit 9c3b9a4926
8 changed files with 137 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files
odoo/addons/base/models/ir_translation.py
+3
View File
@@ -251,6 +251,7 @@ class IrTranslation(models.Model):
:param src: the source of the translation
"""
self._modified_model(name.split(',')[0])
self.flush_model()
# update existing translations
self._cr.execute("""UPDATE ir_translation
@@ -259,6 +260,7 @@ class IrTranslation(models.Model):
RETURNING res_id""",
(value, src, 'translated', lang, tt, name, tuple(ids)))
existing_ids = [row[0] for row in self._cr.fetchall()]
self.invalidate_model(['value', 'src', 'state'])
# create missing translations
self.sudo().create([{
@@ -723,6 +725,7 @@ class IrTranslation(models.Model):
""",
(values[0], values[1], values[2], where[0], where[1], where[2], tuple(values[3]))
)
self.invalidate_model(['value', 'src', 'state'])
@api.model
def translate_fields(self, model, id, field=None):
odoo/addons/base/tests/test_db_cursor.py
+2
View File
@@ -51,6 +51,8 @@ class TestTestCursor(common.TransactionCase):
record.flush_model(['ref'])
def check(self, record, value):
# make sure to fetch the field from the database
record.invalidate_recordset()
self.assertEqual(record.read(['ref'])[0]['ref'], value)
def test_single_cursor(self):
odoo/addons/base/tests/test_ir_actions.py
+2 -2
View File
@@ -514,8 +514,8 @@ class TestCustomFields(common.TransactionCase):
'store': True,
})
# same with a related field, it only takes 5 extra queries
with self.assertQueryCount(query_count + 5):
# same with a related field, it only takes 8 extra queries
with self.assertQueryCount(query_count + 8):
self.env.registry.clear_caches()
self.env['ir.model.fields'].create({
'model_id': model_id,
odoo/addons/base/tests/test_translate.py
+44 -11
View File
@@ -485,18 +485,51 @@ class TestTranslationWrite(TransactionCase):
langs = self.env['res.lang'].get_installed()
self.assertEqual([('en_US', 'English (US)'), ('fr_FR', 'French / Français')], langs,
"Test did not started with expected languages")
self.env['ir.translation'].create({
'type': 'model',
'name': 'res.partner.category,name',
'lang': 'en_US',
'res_id': self.category.id,
'src': 'Reblochon',
'value': 'Translated Name',
'state': 'translated',
})
self.category.with_context(lang='fr_FR').write({'name': 'French Name'})
self.category.with_context(lang='en_US').write({'name': 'English Name'})
category_en = self.category.with_context(lang='en_US')
category_fr = self.category.with_context(lang='fr_FR')
# no translation at first
self.assertEqual(category_fr.name, 'Reblochon')
self.assertFalse(self.env['ir.translation'].search([
('name', '=', 'res.partner.category,name'),
('res_id', '=', self.category.id),
], order='lang'))
# change source
self.category.write({'name': 'Blorb'})
self.assertEqual(category_fr.name, 'Blorb')
self.assertFalse(self.env['ir.translation'].search([
('name', '=', 'res.partner.category,name'),
('res_id', '=', self.category.id),
], order='lang'))
# change source
category_en.write({'name': 'Cheese'})
self.assertEqual(category_fr.name, 'Cheese')
translations = self.env['ir.translation'].search([
('name', '=', 'res.partner.category,name'),
('res_id', '=', self.category.id),
], order='lang')
self.assertRecordValues(translations, [
{'src': 'Cheese', 'value': 'Cheese', 'lang': 'en_US'},
])
# add a translation
category_fr.write({'name': 'French Name'})
self.assertEqual(category_en.name, 'Cheese')
translations = self.env['ir.translation'].search([
('name', '=', 'res.partner.category,name'),
('res_id', '=', self.category.id),
], order='lang')
self.assertRecordValues(translations, [
{'src': 'Cheese', 'value': 'Cheese', 'lang': 'en_US'},
{'src': 'Cheese', 'value': 'French Name', 'lang': 'fr_FR'},
])
# change source
category_en.write({'name': 'English Name'})
self.assertEqual(category_fr.name, 'French Name')
translations = self.env['ir.translation'].search([
('name', '=', 'res.partner.category,name'),
('res_id', '=', self.category.id),
odoo/addons/test_translation_import/tests/test_term_count.py
+1 -1
View File
@@ -230,7 +230,7 @@ class TestTermCount(common.TransactionCase):
trans_count = self.env['ir.translation'].search_count([('lang', '=', 'dot')])
self.assertEqual(trans_count, 1, "The imported translations were not created")
self.env.context = dict(self.env.context, lang="dot")
self.env = self.env(context=dict(self.env.context, lang="dot"))
self.assertEqual(_("Accounting"), "samva", "The code translation was not applied")
def test_export_pollution(self):
odoo/api.py
+9 -2
View File
@@ -701,8 +701,15 @@ class Environment(Mapping):
)
return self.cr.savepoint()
def invalidate_all(self):
""" Invalidate the cache of all records. """
def invalidate_all(self, flush=True):
""" Invalidate the cache of all records.
:param flush: whether pending updates should be flushed before invalidation.
It is ``True`` by default, which ensures cache consistency.
Do not use this parameter unless you know what you are doing.
"""
if flush:
self.flush_all()
self.cache.invalidate()
def _recompute_all(self):
odoo/models.py
+75 -60
View File
@@ -3760,74 +3760,77 @@ class BaseModel(metaclass=MetaModel):
if func._ondelete or not self._context.get(MODULE_UNINSTALL_FLAG):
func(self)
# mark fields that depend on 'self' to recompute them after 'self' has
# been deleted (like updating a sum of lines after deleting one line)
# TOFIX: this avoids an infinite loop when trying to recompute a
# field, which triggers the recomputation of another field using the
# same compute function, which then triggers again the computation
# of those two fields
for field in self._fields.values():
self.env.remove_to_compute(field, self)
self.env.flush_all()
self.modified(self._fields, before=True)
with self.env.norecompute():
cr = self._cr
Data = self.env['ir.model.data'].sudo().with_context({})
Defaults = self.env['ir.default'].sudo()
Property = self.env['ir.property'].sudo()
Attachment = self.env['ir.attachment'].sudo()
ir_model_data_unlink = Data
ir_attachment_unlink = Attachment
cr = self._cr
Data = self.env['ir.model.data'].sudo().with_context({})
Defaults = self.env['ir.default'].sudo()
Property = self.env['ir.property'].sudo()
Attachment = self.env['ir.attachment'].sudo()
ir_property_unlink = Property
ir_model_data_unlink = Data
ir_attachment_unlink = Attachment
# TOFIX: this avoids an infinite loop when trying to recompute a
# field, which triggers the recomputation of another field using the
# same compute function, which then triggers again the computation
# of those two fields
for field in self._fields.values():
self.env.remove_to_compute(field, self)
for sub_ids in cr.split_for_in_conditions(self.ids):
records = self.browse(sub_ids)
for sub_ids in cr.split_for_in_conditions(self.ids):
# Check if the records are used as default properties.
refs = ['%s,%s' % (self._name, i) for i in sub_ids]
if Property.search([('res_id', '=', False), ('value_reference', 'in', refs)], limit=1):
raise UserError(_('Unable to delete this document because it is used as a default property'))
# Check if the records are used as default properties.
refs = [f'{self._name},{id_}' for id_ in sub_ids]
if Property.search([('res_id', '=', False), ('value_reference', 'in', refs)], limit=1):
raise UserError(_('Unable to delete this document because it is used as a default property'))
# Delete the records' properties.
Property.search([('res_id', 'in', refs)]).unlink()
# Delete the records' properties.
ir_property_unlink |= Property.search([('res_id', 'in', refs)])
query = "DELETE FROM %s WHERE id IN %%s" % self._table
cr.execute(query, (sub_ids,))
# mark fields that depend on 'self' to recompute them after 'self' has
# been deleted (like updating a sum of lines after deleting one line)
with self.env.protecting(self._fields.values(), records):
self.modified(self._fields, before=True)
# Removing the ir_model_data reference if the record being deleted
# is a record created by xml/csv file, as these are not connected
# with real database foreign keys, and would be dangling references.
#
# Note: the following steps are performed as superuser to avoid
# access rights restrictions, and with no context to avoid possible
# side-effects during admin calls.
data = Data.search([('model', '=', self._name), ('res_id', 'in', sub_ids)])
if data:
ir_model_data_unlink |= data
query = f'DELETE FROM "{self._table}" WHERE id IN %s'
cr.execute(query, (sub_ids,))
# For the same reason, remove the defaults having some of the
# records as value
Defaults.discard_records(self.browse(sub_ids))
# Removing the ir_model_data reference if the record being deleted
# is a record created by xml/csv file, as these are not connected
# with real database foreign keys, and would be dangling references.
#
# Note: the following steps are performed as superuser to avoid
# access rights restrictions, and with no context to avoid possible
# side-effects during admin calls.
data = Data.search([('model', '=', self._name), ('res_id', 'in', sub_ids)])
ir_model_data_unlink |= data
# For the same reason, remove the relevant records in ir_attachment
# (the search is performed with sql as the search method of
# ir_attachment is overridden to hide attachments of deleted
# records)
query = 'SELECT id FROM ir_attachment WHERE res_model=%s AND res_id IN %s'
cr.execute(query, (self._name, sub_ids))
attachments = Attachment.browse([row[0] for row in cr.fetchall()])
if attachments:
ir_attachment_unlink |= attachments.sudo()
# For the same reason, remove the defaults having some of the
# records as value
Defaults.discard_records(records)
# invalidate the *whole* cache, since the orm does not handle all
# changes made in the database, like cascading delete!
self.env.invalidate_all()
if ir_model_data_unlink:
ir_model_data_unlink.unlink()
if ir_attachment_unlink:
ir_attachment_unlink.unlink()
# DLE P93: flush after the unlink, for recompute fields depending on
# the modified of the unlink
self.env.flush_all()
# For the same reason, remove the relevant records in ir_attachment
# (the search is performed with sql as the search method of
# ir_attachment is overridden to hide attachments of deleted
# records)
query = 'SELECT id FROM ir_attachment WHERE res_model=%s AND res_id IN %s'
cr.execute(query, (self._name, sub_ids))
ir_attachment_unlink |= Attachment.browse(row[0] for row in cr.fetchall())
# invalidate the *whole* cache, since the orm does not handle all
# changes made in the database, like cascading delete!
self.env.invalidate_all(flush=False)
if ir_property_unlink:
ir_property_unlink.unlink()
if ir_model_data_unlink:
ir_model_data_unlink.unlink()
if ir_attachment_unlink:
ir_attachment_unlink.unlink()
# DLE P93: flush after the unlink, for recompute fields depending on
# the modified of the unlink
self.env.flush_all()
# auditing: deletions are infrequent and leave no trace in the database
_unlink.info('User #%s deleted %s records with IDs: %r', self._uid, self._name, self.ids)
@@ -4666,6 +4669,8 @@ class BaseModel(metaclass=MetaModel):
:return: the qualified field name (or expression) to use for ``field``
"""
if self.env.lang:
# for the COALESCE to work properly, the column must be flushed
self.flush_model([field])
alias = query.left_join(
table_alias, 'id', 'ir_translation', 'res_id', field,
extra='"{rhs}"."type" = \'model\' AND "{rhs}"."name" = %s AND "{rhs}"."lang" = %s AND "{rhs}"."value" != %s',
@@ -6151,22 +6156,32 @@ class BaseModel(metaclass=MetaModel):
else:
self.env.invalidate_all()
def invalidate_model(self, fnames=None):
def invalidate_model(self, fnames=None, flush=True):
""" Invalidate the cache of all records of ``self``'s model, when the
cached values no longer correspond to the database values. If the
parameter is given, only the given fields are invalidated from cache.
:param fnames: optional iterable of field names to invalidate
:param flush: whether pending updates should be flushed before invalidation.
It is ``True`` by default, which ensures cache consistency.
Do not use this parameter unless you know what you are doing.
"""
if flush:
self.flush_model(fnames)
self._invalidate_cache(fnames)
def invalidate_recordset(self, fnames=None):
def invalidate_recordset(self, fnames=None, flush=True):
""" Invalidate the cache of the records in ``self``, when the cached
values no longer correspond to the database values. If the parameter
is given, only the given fields on ``self`` are invalidated from cache.
:param fnames: optional iterable of field names to invalidate
:param flush: whether pending updates should be flushed before invalidation.
It is ``True`` by default, which ensures cache consistency.
Do not use this parameter unless you know what you are doing.
"""
if flush:
self.flush_recordset(fnames)
self._invalidate_cache(fnames, self._ids)
def _invalidate_cache(self, fnames=None, ids=None):
odoo/modules/registry.py
+1 -6
View File
@@ -245,6 +245,7 @@ class Registry(Mapping):
This must be called after loading modules and before using the ORM.
"""
env = odoo.api.Environment(cr, SUPERUSER_ID, {})
env.invalidate_all()
# Uninstall registry hooks. Because of the condition, this only happens
# on a fully loaded registry, and not on a registry being loaded.
@@ -258,12 +259,6 @@ class Registry(Mapping):
lazy_property.reset_all(self)
self.registry_invalidated = True
if env.all.tocompute:
_logger.error(
"Remaining fields to compute before setting up registry: %s",
env.all.tocompute, stack_info=True,
)
# we must setup ir.model before adding manual fields because _add_manual_models may
# depend on behavior that is implemented through overrides, such as is_mail_thread which
# is implemented through an override to env['ir.model']._instanciate