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

[FIX] models: make onchange return a smaller diff

Browse Source 
Large diffs (specifically in x2many fields) cause performance issues.  They
overload the web client, which considers all returned fields as dirty.

closes odoo/odoo#27941
This commit is contained in:
Raphael Collet
2018-10-23 14:25:38 +00:00
parent 4d8b0b723b
commit 828b76d578
2 changed files with 130 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files
odoo/addons/test_new_api/tests/test_onchange.py
+41 -40
View File
@@ -104,12 +104,12 @@ class TestOnChange(common.TransactionCase):
def test_onchange_one2many(self):
""" test the effect of onchange() on one2many fields """
BODY = "What a beautiful day!"
USER = self.env.user
# create an independent message
message = self.Message.create({'body': BODY})
self.assertEqual(message.name, "[%s] %s" % ('', USER.name))
message1 = self.Message.create({'body': "ABC"})
message2 = self.Message.create({'body': "ABC"})
self.assertEqual(message1.name, "[%s] %s" % ('', USER.name))
field_onchange = self.Discussion._onchange_spec()
self.assertEqual(field_onchange.get('name'), '1')
@@ -126,12 +126,14 @@ class TestOnChange(common.TransactionCase):
'moderator': False,
'participants': [],
'messages': [
(4, message.id),
(4, message1.id),
(4, message2.id),
(1, message2.id, {'body': "XYZ"}),
(0, 0, {
'name': "[%s] %s" % ('', USER.name),
'body': BODY,
'body': "ABC",
'author': USER.id,
'size': len(BODY),
'size': 3,
'important': False,
}),
],
@@ -139,20 +141,20 @@ class TestOnChange(common.TransactionCase):
self.env.cache.invalidate()
result = self.Discussion.onchange(values, 'name', field_onchange)
self.assertIn('messages', result['value'])
self.assertItemsEqual(result['value']['messages'], [
self.assertEqual(result['value']['messages'], [
(5,),
(1, message.id, {
(1, message1.id, {
'name': "[%s] %s" % ("Foo", USER.name),
'body': message.body,
'author': message.author.name_get()[0],
'size': message.size,
'important': message.important,
}),
(1, message2.id, {
'name': "[%s] %s" % ("Foo", USER.name),
'body': "XYZ", # this must be sent back
}),
(0, 0, {
'name': "[%s] %s" % ("Foo", USER.name),
'body': BODY,
'body': "ABC",
'author': USER.name_get()[0],
'size': len(BODY),
'size': 3,
'important': False,
}),
])
@@ -163,7 +165,8 @@ class TestOnChange(common.TransactionCase):
result = self.Discussion.with_context(generate_dummy_message=True).onchange(values, 'name', one_level_fields)
self.assertEqual(result['value']['messages'], [
(5,),
(4, message.id),
(4, message1.id),
(4, message2.id),
(0, 0, {}),
(0, 0, {}),
])
@@ -256,8 +259,7 @@ class TestOnChange(common.TransactionCase):
'lines': [
(5,),
(1, line1.id, {'name': partner2.name,
'partner': (partner2.id, partner2.name),
'tags': [(5,)]}),
'partner': (partner2.id, partner2.name)}),
(0, 0, {'name': partner2.name,
'partner': (partner2.id, partner2.name),
'tags': [(5,)]}),
@@ -276,18 +278,30 @@ class TestOnChange(common.TransactionCase):
self.env.cache.invalidate()
result = multi.onchange(values, 'partner', field_onchange)
self.assertEqual(result['value'], {
expected_value = {
'name': partner2.name,
'lines': [
(5,),
(1, line1.id, {'name': partner2.name,
'partner': (partner2.id, partner2.name),
'tags': [(5,)]}),
'partner': (partner2.id, partner2.name)}),
(0, 0, {'name': partner2.name,
'partner': (partner2.id, partner2.name),
'tags': [(5,), (0, 0, {'name': 'Tag'})]}),
],
})
}
self.assertEqual(result['value'], expected_value)
# ensure ID is not returned when asked and a many2many record is set to be created
self.env.cache.invalidate()
result = multi.onchange(values, 'partner', dict(field_onchange, **{'lines.tags.id': None}))
self.assertEqual(result['value'], expected_value)
# ensure inverse of one2many field is not returned
self.env.cache.invalidate()
result = multi.onchange(values, 'partner', dict(field_onchange, **{'lines.multi': None}))
self.assertEqual(result['value'], expected_value)
def test_onchange_specific(self):
""" test the effect of field-specific onchange method """
@@ -319,8 +333,7 @@ class TestOnChange(common.TransactionCase):
self.assertIn('participants', result['value'])
self.assertItemsEqual(
result['value']['participants'],
[(5,)] + [(1, user.id, {'display_name': user.display_name})
for user in discussion.participants + demo],
[(5,)] + [(4, user.id) for user in discussion.participants + demo],
)
def test_onchange_default(self):
@@ -356,6 +369,8 @@ class TestOnChange(common.TransactionCase):
self.assertEqual(len(discussion.messages), 3)
messages = [(4, msg.id) for msg in discussion.messages]
messages[0] = (1, messages[0][1], {'body': 'test onchange'})
lines = ["%s:%s" % (m.name, m.body) for m in discussion.messages]
lines[0] = "%s:%s" % (discussion.messages[0].name, 'test onchange')
values = {
'name': discussion.name,
'moderator': demo.id,
@@ -366,8 +381,7 @@ class TestOnChange(common.TransactionCase):
}
result = discussion.onchange(values, 'messages', field_onchange)
self.assertIn('message_concat', result['value'])
self.assertEqual(result['value']['message_concat'],
"\n".join(["%s:%s" % (m.name, m.body) for m in discussion.messages]))
self.assertEqual(result['value']['message_concat'], "\n".join(lines))
def test_onchange_one2many_with_domain_on_related_field(self):
""" test the value of the one2many field when defined with a domain on a related field"""
@@ -413,28 +427,15 @@ class TestOnChange(common.TransactionCase):
'categories': [(4, cat.id) for cat in discussion.categories],
'messages': [(4, msg.id) for msg in discussion.messages],
'participants': [(4, usr.id) for usr in discussion.participants],
'message_changes': 0,
'important_messages': [(4, msg.id) for msg in discussion.important_messages],
'important_emails': [(4, eml.id) for eml in discussion.important_emails],
}
self.env.cache.invalidate()
result = discussion.onchange(values, 'name', field_onchange)
# When one2many domain contains non-computed field, things are ok
self.assertEqual(result['value']['important_messages'],
[(5,)] + [(4, msg.id) for msg in discussion.important_messages])
# But here with commit 5676d81, we get value of: [(2, email.id)]
self.assertEqual(
result['value']['important_emails'],
[(5,),
(1, email.id, {
'name': u'[Foo Bar] %s' % USER.name,
'body': email.body,
'author': USER.name_get()[0],
'important': True,
'email_to': demo.email,
'size': email.size,
})]
[(5,), (1, email.id, {'name': u'[Foo Bar] %s' % USER.name})],
)
def test_onchange_related(self):
odoo/models.py
+89 -40
View File
@@ -5304,31 +5304,98 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
if not all(name in self._fields for name in names):
return {}
# filter out keys in field_onchange that do not refer to actual fields
dotnames = []
for dotname in field_onchange:
try:
model = self.browse()
for name in dotname.split('.'):
model = model[name]
dotnames.append(dotname)
except Exception:
pass
def PrefixTree(model, dotnames):
""" Return a prefix tree for sequences of field names. """
if not dotnames:
return {}
# group dotnames by prefix
suffixes = defaultdict(list)
for dotname in dotnames:
# name, *names = dotname.split('.', 1)
names = dotname.split('.', 1)
name = names.pop(0)
suffixes[name].extend(names)
# fill in prefix tree in fields order
tree = OrderedDict()
for name, field in model._fields.items():
if name in suffixes:
tree[name] = subtree = PrefixTree(model[name], suffixes[name])
if subtree and field.type == 'one2many':
subtree.pop(field.inverse_name, None)
return tree
class Snapshot(dict):
""" A dict with the values of a record, following a prefix tree. """
__slots__ = ()
def __init__(self, record, tree):
# put record in dict to include it when comparing snapshots
super(Snapshot, self).__init__({'<record>': record, '<tree>': tree})
for name, subnames in tree.items():
# x2many fields are serialized as a list of line snapshots
self[name] = (
[Snapshot(line, subnames) for line in record[name]]
if subnames else record[name]
)
def diff(self, other):
""" Return the values in ``self`` that differ from ``other``.
Requires record cache invalidation for correct output!
"""
record = self['<record>']
result = {}
for name, subnames in self['<tree>'].items():
if (name == 'id') or (other.get(name) == self[name]):
continue
if not subnames:
field = record._fields[name]
result[name] = field.convert_to_onchange(self[name], record, {})
else:
# x2many fields: serialize value as commands
result[name] = commands = [(5,)]
for line_snapshot in self[name]:
line = line_snapshot['<record>']
if not line.id:
# new line: send diff from scratch
line_diff = line_snapshot.diff({})
commands.append((0, line.id.ref or 0, line_diff))
else:
# existing line: send diff from database
# (requires a clean record cache!)
line_diff = line_snapshot.diff(Snapshot(line, subnames))
if line_diff:
commands.append((1, line.id, line_diff))
else:
commands.append((4, line.id))
return result
nametree = PrefixTree(self.browse(), field_onchange)
# prefetch x2many lines without data (for the initial snapshot)
for name, subnames in nametree.items():
if subnames and values.get(name):
# retrieve all ids in commands, and read the expected fields
line_ids = []
for cmd in values[name]:
if cmd[0] in (1, 4):
line_ids.append(cmd[1])
elif cmd[0] == 6:
line_ids.extend(cmd[2])
lines = self.browse()[name].browse(line_ids)
lines.read(list(subnames), load='_classic_write')
# create a new record with values, and attach ``self`` to it
with env.do_in_onchange():
record = self.new(values)
values = {name: record[name] for name in record._cache}
# attach ``self`` with a different context (for cache consistency)
record._origin = self.with_context(__onchange=True)
# load fields on secondary records, to avoid false changes
# make a snapshot based on the initial values of record
with env.do_in_onchange():
for dotname in dotnames:
record.mapped(dotname)
snapshot0 = snapshot1 = Snapshot(record, nametree)
# determine which field(s) should be triggered an onchange
todo = list(names) or list(values)
todo = list(names or nametree)
done = set()
# dummy assignment: trigger invalidations on the record
@@ -5344,7 +5411,6 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
record[name] = value
result = {'warnings': OrderedSet()}
dirty = set()
# process names in order (or the keys of values if no name given)
while todo:
@@ -5358,34 +5424,17 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
if field_onchange.get(name):
record._onchange_eval(name, field_onchange[name], result)
# force re-evaluation of function fields on secondary records
for dotname in dotnames:
record.mapped(dotname)
# make a snapshot (this forces evaluation of computed fields)
snapshot1 = Snapshot(record, nametree)
# determine which fields have been modified
for name, oldval in values.items():
field = self._fields[name]
newval = record[name]
if newval != oldval or (
field.type in ('one2many', 'many2many') and newval._is_dirty()
):
for name in nametree:
if snapshot1[name] != snapshot0[name]:
todo.append(name)
dirty.add(name)
# determine subfields for field.convert_to_onchange() below
Tree = lambda: defaultdict(Tree)
subnames = Tree()
for dotname in dotnames:
subtree = subnames
for name in dotname.split('.'):
subtree = subtree[name]
# collect values from dirty fields
with env.do_in_onchange():
result['value'] = {
name: self._fields[name].convert_to_onchange(record[name], record, subnames[name])
for name in dirty
}
# determine values that have changed by comparing snapshots
self.invalidate_cache()
result['value'] = snapshot1.diff(snapshot0)
# format warnings
warnings = result.pop('warnings')