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

[IMP] base: do not modify the cache when reading properties fields in batch

Browse Source 
Purpose
=======
Do not modify the cache in order to improve the performance when we
read in batch. Instead we create a new batched method "convert_to_read"
that check existence in batch, and generate a dict with the result.

Now, all the properties field checks (many2one existence, selection option
still exists, tag value stiff exist, etc) and done in "convert_to_read".

It means that doing `record.properties` won't do all those checks.

Having the batched field fetch at the convert_to_record required too
many changes for the scope of this task, so "record.properties" having
the cache values unchecked is an acceptable tradeoff currently, hence
we batch convert_to_read.

Task-2980121

Part-of: odoo/odoo#101901
This commit is contained in:
std-odoo
2023-03-07 01:59:04 +01:00
parent df997b1d01
commit da48700b59
3 changed files with 80 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files
odoo/addons/test_new_api/tests/test_properties.py
+16 -8
View File
@@ -592,7 +592,7 @@ class PropertiesCase(TransactionCase):
# 1 query to read the field
# 1 query to read the definition
# 2 queries to check if the many2one still exists / name_get
self.assertFalse(self.message_2.attributes[0]['value'])
self.assertFalse(self.message_2.read(['attributes'])[0]['attributes'][0]['value'])
# remove the partner, and use the read method
self.message_2.attributes = [{
@@ -844,9 +844,9 @@ class PropertiesCase(TransactionCase):
# the value must remain in the database until the next write on the child
self.assertEqual(self._get_sql_properties(message), {'my_tags': ['be', 'de']})
attributes = message.read(['attributes'])[0]['attributes']
self.assertEqual(
message.attributes[0]['value'],
attributes[0]['value'],
['be'],
msg='The tag has been removed on the definition, should be removed when reading the child')
self.assertEqual(
@@ -854,7 +854,7 @@ class PropertiesCase(TransactionCase):
[['be', 'BE', 1], ['fr', 'FR', 2], ['it', 'IT', 1]])
# next write on the child must update the value
message.attributes = message.attributes
message.attributes = message.read(['attributes'])[0]['attributes']
self.assertEqual(self._get_sql_properties(message), {'my_tags': ['be']})
@@ -890,7 +890,7 @@ class PropertiesCase(TransactionCase):
'comodel': 'test_new_api.partner',
}]
with self.assertQueryCount(2):
with self.assertQueryCount(5):
self.message_1.attributes = [
{
"name": "moderator_partner_ids",
@@ -900,11 +900,13 @@ class PropertiesCase(TransactionCase):
"value": partners[:10].name_get(),
}
]
self.assertEqual(self.message_1.attributes[0]['value'], partners[:10].ids)
attributes = self.message_1.read(['attributes'], load=None)[0]['attributes']
self.assertEqual(attributes[0]['value'], partners[:10].ids)
partners[:5].unlink()
with self.assertQueryCount(5):
self.assertEqual(self.message_1.attributes[0]['value'], partners[5:10].ids)
with self.assertQueryCount(4):
attributes = self.message_1.read(['attributes'], load=None)[0]['attributes']
self.assertEqual(attributes[0]['value'], partners[5:10].ids)
partners[5].unlink()
with self.assertQueryCount(5):
@@ -1045,6 +1047,12 @@ class PropertiesCase(TransactionCase):
]
self.message_1.flush_recordset()
last_message_id = self.env['test_new_api.message'].search([], order="id DESC", limit=1).id
# based on batch optimization, _read_format should not crash on non existing records
values = self.env['test_new_api.message'].browse((self.message_1.id, last_message_id + 1))._read_format(['attributes'])
self.assertEqual(len(values), 1)
self.assertEqual(values[0]['id'], self.message_1.id)
def test_properties_field_change_definition(self):
"""Test the behavior of the field when changing the definition."""
odoo/fields.py
+44 -51
View File
@@ -3290,7 +3290,6 @@ class Properties(Field):
assert isinstance(value, dict), f"Wrong type {value!r}"
value = self._dict_to_list(value, definition)
self._parse_json_types(value, record.env)
return value
@@ -3312,10 +3311,22 @@ class Properties(Field):
# }]
#
def convert_to_read(self, value, record, use_name_get=True):
return self.convert_to_read_multi([value], record, use_name_get)[0]
def convert_to_read_multi(self, values, records, use_name_get=True):
assert len(values) == len(records)
res_ids_per_model = self._get_res_ids_per_model(records, values, use_name_get)
# value is in record format
for value in values:
self._parse_json_types(value, records.env, res_ids_per_model)
if use_name_get:
self._add_display_name(value, record.env)
return value
for value in values:
self._add_display_name(value, records.env)
return values
def convert_to_write(self, value, record):
"""If we write a list on the child, update the definition record."""
@@ -3330,37 +3341,29 @@ class Properties(Field):
self._add_display_name(value, record.env)
return value
def read(self, records):
def _get_res_ids_per_model(self, records, values_list, use_name_get=True):
"""Read everything needed in batch for the given records.
To retrieve relational properties names, or to check their existence,
we need to do some SQL queries. To reduce the number of queries when we read
in batch, we put in cache everything needed before calling
in batch, we prefetch everything needed before calling
convert_to_record / convert_to_read.
"""
definition_records_map = {
record: record[self.definition_record][self.definition_record_field]
for record in records
}
Return a dict {model: record_ids} that contains
the existing ids for each needed models.
"""
# ids per model we need to fetch in batch to put in cache
ids_per_model = defaultdict(OrderedSet)
records_cached_values = list(records.env.cache.get_values(records, self))
for record, record_values in zip(records, records_cached_values):
definition = definition_records_map.get(record)
if not record_values or not definition:
continue
for property_definition in definition:
for record, record_values in zip(records, values_list):
for property_definition in record_values:
comodel = property_definition.get('comodel')
type_ = property_definition.get('type')
name = property_definition.get('name')
if not comodel or type_ not in ('many2one', 'many2many') or name not in record_values:
continue
property_value = property_definition.get('value') or []
default = property_definition.get('default') or []
property_value = record_values[name] or []
if type_ not in ('many2one', 'many2many') or comodel not in records.env:
continue
if type_ == 'many2one':
default = [default] if default else []
@@ -3370,38 +3373,20 @@ class Properties(Field):
ids_per_model[comodel].update(property_value)
# check existence and pre-fetch in batch
existing_ids_per_model = {}
res_ids_per_model = {}
for model, ids in ids_per_model.items():
recs = records.env[model].browse(ids).exists()
existing_ids_per_model[model] = set(recs.ids)
for record in recs:
# read a field to pre-fetch the recordset
try:
record.display_name
except AccessError:
pass
res_ids_per_model[model] = set(recs.ids)
# update the cache and remove non-existing ids
for record, record_values in zip(records, records_cached_values):
definition = definition_records_map.get(record)
if not record_values or not definition:
continue
if use_name_get:
for record in recs:
# read a field to pre-fetch the recordset
try:
record.display_name
except AccessError:
pass
for property_definition in definition:
comodel = property_definition.get('comodel')
type_ = property_definition.get('type')
name = property_definition.get('name')
if not comodel or type_ not in ('many2one', 'many2many') or not record_values.get(name):
continue
property_value = record_values[name]
if type_ == 'many2one':
record_values[name] = property_value if property_value in existing_ids_per_model[comodel] else False
else:
record_values[name] = [id_ for id_ in property_value if id_ in existing_ids_per_model[comodel]]
records.env.cache.update(record, self, [record_values], check_dirty=False)
return res_ids_per_model
def write(self, records, value):
"""Check if the properties definition has been changed.
@@ -3591,7 +3576,7 @@ class Properties(Field):
definition['name'] = str(uuid.uuid4()).replace('-', '')[:16]
@classmethod
def _parse_json_types(cls, values_list, env):
def _parse_json_types(cls, values_list, env, res_ids_per_model):
"""Parse the value stored in the JSON.
Check for records existence, if we removed a selection option, ...
@@ -3633,6 +3618,9 @@ class Properties(Field):
if not isinstance(property_value, int):
raise ValueError(f'Wrong many2one value: {property_value!r}.')
if property_value not in res_ids_per_model[res_model]:
property_value = False
elif property_type == 'many2many' and property_value and res_model in env:
if not is_list_of(property_value, int):
raise ValueError(f'Wrong many2many value: {property_value!r}.')
@@ -3641,6 +3629,11 @@ class Properties(Field):
# remove duplicated value and preserve order
property_value = list(dict.fromkeys(property_value))
property_value = [
id_ for id_ in property_value
if id_ in res_ids_per_model[res_model]
]
property_definition['value'] = property_value
@classmethod
odoo/models.py
+20 -1
View File
@@ -3144,13 +3144,32 @@ class BaseModel(metaclass=MetaModel):
The output format is the one expected from the `read` method, which uses
this method as its implementation for formatting values.
For the properties fields, call convert_to_read_multi instead of convert_to_read
to prepare everything (record existences, display name, etc) in batch.
The current method is different from `read` because it retrieves its
values from the cache without doing a query when it is avoidable.
"""
data = [(record, {'id': record._ids[0]}) for record in self]
use_name_get = (load == '_classic_read')
for name in fnames:
convert = self._fields[name].convert_to_read
field = self._fields[name]
if field.type == 'properties':
values_list = []
records = []
for record, vals in data:
try:
values_list.append(record[name])
records.append(record.id)
except MissingError:
vals.clear()
results = field.convert_to_read_multi(values_list, self.browse(records), use_name_get)
for record_read_vals, convert_result in zip(data, results):
record_read_vals[1][name] = convert_result
continue
convert = field.convert_to_read
for record, vals in data:
# missing records have their vals empty
if not vals: