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

[IMP] core, base: avoid multiple xids when importing records w/ inherits

Browse Source 
Before this change, we create an xid for every parent of a record
being imported, regardless of whether it already has an xid, or if
it's being created implicitly through the child.

This generates unnecessary extra xids on pre-existing objects
e.g. update 5 product variants -> the product gets 5 new xids despite
already having one.

We should *only* set a xid on parent records which are being
implicitly created by the creation of a child with a specified
xid. That is, we should never set a xid on the parent if it exists
before the child is created.

Update _process_end to try and see if "non-loaded" xids correspond to
an automatically generated "parent" xid: we're still setting a xid on
implicitly created records (if the child is created with a xid) so
they're properly removed if e.g. the module is uninstalled, but
because we're only doing so at creation these xids will not be visited
during update and _process_end will try to delete them.

A special case can be added to check that "unknown" parent xids don't
have children which _inherit them, in which case we want to protect them.

Task 2251039

closes odoo/odoo#53283

Related: odoo/enterprise#12023
Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
This commit is contained in:
Xavier Morel
2020-07-27 06:32:59 +00:00
parent dfda8b010b
commit ef709fd92b
7 changed files with 252 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/fleet/data/fleet_demo.xml
+1 -1
View File
@@ -463,7 +463,7 @@
<field name="color">Black</field>
<field name="location">Grand-Rosiere</field>
<field name="doors">5</field>
<field name="driver_id" ref="base.user_demo_res_partner" />
<field name="driver_id" ref="base.partner_demo" />
<field name="acquisition_date" eval="(DateTime.now() - timedelta(days=336)).strftime('%Y-%m-%d')" />
<field name="state_id" ref="fleet_vehicle_state_registered"/>
<field name="odometer_unit">kilometers</field>
odoo/addons/base/models/ir_model.py
+56 -41
View File
@@ -1970,12 +1970,6 @@ class IrModelData(models.Model):
return result
def _generate_xmlids(self, xml_id, model):
""" Return all the XML ids to create for the given model. """
yield xml_id
for parent_model in model._inherits:
yield '%s_%s' % (xml_id, parent_model.replace('.', '_'))
@api.model
def _update_xmlids(self, data_list, update=False):
""" Create or update the given XML ids.
@@ -1992,14 +1986,6 @@ class IrModelData(models.Model):
prefix, suffix = data['xml_id'].split('.', 1)
record = data['record']
noupdate = bool(data.get('noupdate'))
# First create XML ids for parent records, then create XML id for
# record. The order reflects their actual creation order. This order
# is relevant for the uninstallation process: the record must be
# deleted before its parent records.
for parent_model, parent_field in record._inherits.items():
parent = record[parent_field]
puffix = suffix + '_' + parent_model.replace('.', '_')
rows.add((prefix, puffix, parent._name, parent.id, noupdate))
rows.add((prefix, suffix, record._name, record.id, noupdate))
for sub_rows in self.env.cr.split_for_in_conditions(rows):
@@ -2036,8 +2022,6 @@ class IrModelData(models.Model):
record = self.xmlid_to_object(xml_id)
if record:
self.pool.loaded_xmlids.add(xml_id)
for parent_model, parent_field in record._inherits.items():
self.pool.loaded_xmlids.add(xml_id + '_' + parent_model.replace('.', '_'))
return record
@api.model
@@ -2176,32 +2160,63 @@ class IrModelData(models.Model):
"""
self._cr.execute(query, (tuple(modules), True))
for (id, xmlid, model, res_id) in self._cr.fetchall():
if xmlid not in loaded_xmlids:
if model in self.env:
if self.search_count([
("model", "=", model),
("res_id", "=", res_id),
("id", "!=", id),
("id", "not in", bad_imd_ids),
]):
# another external id is still linked to the same record, only deleting the old imd
bad_imd_ids.append(id)
continue
if xmlid in loaded_xmlids:
continue
_logger.info('Deleting %s@%s (%s)', res_id, model, xmlid)
record = self.env[model].browse(res_id)
if record.exists():
module = xmlid.split('.', 1)[0]
record = record.with_context(module=module)
if record._name == 'ir.model.fields' and not module.startswith('test_'):
_logger.warning(
"Deleting field %s.%s (hint: fields should be"
" explicitly removed by an upgrade script)",
record.model, record.name,
)
record.unlink()
else:
bad_imd_ids.append(id)
Model = self.env.get(model)
if Model is None:
continue
# when _inherits parents are implicitly created we give them an
# external id (if their descendant has one) in order to e.g.
# properly remove them when the module is deleted, however this
# generated id is *not* provided during update yet we don't want to
# try and remove either the xid or the record, so check if the
# record has a child we've just updated
keep = False
for inheriting in (self.env[m] for m in Model._inherits_children):
# ignore mixins
if inheriting._abstract:
continue
parent_field = inheriting._inherits[model]
children = inheriting.with_context(active_test=False).search([(parent_field, '=', res_id)])
children_xids = {
xid
for xids in (children and children._get_external_ids().values())
for xid in xids
}
if children_xids & loaded_xmlids:
# at least one child was loaded
keep = True
break
if keep:
continue
# if the record has other associated xids, only remove the xid
if self.search_count([
("model", "=", model),
("res_id", "=", res_id),
("id", "!=", id),
("id", "not in", bad_imd_ids),
]):
bad_imd_ids.append(id)
continue
_logger.info('Deleting %s@%s (%s)', res_id, model, xmlid)
record = Model.browse(res_id)
if record.exists():
module = xmlid.split('.', 1)[0]
record = record.with_context(module=module)
if record._name == 'ir.model.fields' and not module.startswith('test_'):
_logger.warning(
"Deleting field %s.%s (hint: fields should be"
" explicitly removed by an upgrade script)",
record.model, record.name,
)
record.unlink()
else:
bad_imd_ids.append(id)
if bad_imd_ids:
self.browse(bad_imd_ids).unlink()
odoo/addons/base/tests/test_ir_model.py
+9 -8
View File
@@ -142,14 +142,15 @@ class TestXMLID(TransactionCase):
def test_create_xmlids(self):
# create users and assign them xml ids
foo, bar = self.env['res.users'].create([
{'name': 'Foo', 'login': 'foo'},
{'name': 'Bar', 'login': 'bar'},
])
self.env['ir.model.data']._update_xmlids([
dict(xml_id='test_convert.foo', record=foo, noupdate=True),
dict(xml_id='test_convert.bar', record=bar, noupdate=True),
])
foo, bar = self.env['res.users']._load_records([{
'xml_id': 'test_convert.foo',
'values': {'name': 'Foo', 'login': 'foo'},
'noupdate': True,
}, {
'xml_id': 'test_convert.bar',
'values': {'name': 'Bar', 'login': 'bar'},
'noupdate': True,
}])
self.assertEqual(foo, self.env.ref('test_convert.foo', raise_if_not_found=False))
self.assertEqual(bar, self.env.ref('test_convert.bar', raise_if_not_found=False))
odoo/addons/test_impex/ir.model.access.csv
+2
View File
@@ -24,3 +24,5 @@ access_export_many2many_other,access_export_many2many_other,model_export_many2ma
access_export_selection_withdefault,access_export_selection_withdefault,model_export_selection_withdefault,,1,1,1,1
access_export_one2many_recursive,access_export_one2many_recursive,model_export_one2many_recursive,,1,1,1,1
access_export_unique,access_export_unique,model_export_unique,,1,1,1,1
access_export_inherits_parent,access_export_inherits_parent,model_export_inherits_parent,,1,1,1,1
access_export_inherits_child,access_export_inherits_child,model_export_inherits_child,,1,1,1,1
1 id name model_id:id group_id:id perm_read perm_write perm_create perm_unlink
24 access_export_selection_withdefault access_export_selection_withdefault model_export_selection_withdefault 1 1 1 1
25 access_export_one2many_recursive access_export_one2many_recursive model_export_one2many_recursive 1 1 1 1
26 access_export_unique access_export_unique model_export_unique 1 1 1 1
27 access_export_inherits_parent access_export_inherits_parent model_export_inherits_parent 1 1 1 1
28 access_export_inherits_child access_export_inherits_child model_export_inherits_child 1 1 1 1
odoo/addons/test_impex/models.py
+12
View File
@@ -164,3 +164,15 @@ class OnlyOne(models.Model):
('value_unique', 'unique (value)', "The value must be unique"),
('pair_unique', 'unique (value2, value3)', "The values must be unique"),
]
class InheritsParent(models.Model):
_name = _description = 'export.inherits.parent'
value_parent = fields.Integer()
class InheritsChild(models.Model):
_name = _description = 'export.inherits.child'
_inherits = {'export.inherits.parent': 'parent_id'}
parent_id = fields.Many2one('export.inherits.parent', required=True, ondelete='cascade')
value = fields.Integer()
odoo/addons/test_impex/tests/test_load.py
+156
View File
@@ -1255,3 +1255,159 @@ class test_unique(ImporterCase):
m.group(2).split(', '),
['Value2', 'Value3']
)
class test_inherits(ImporterCase):
""" The import process should only assign a new xid (derived from the
childs') if the child is being created and triggers the creation of the
parent
"""
model_name = 'export.inherits.child'
def test_create_no_parent(self):
r = self.import_(['id', 'value_parent', 'value'], [
['xxx.child', '0', '1'],
])
rec = self.env[self.model_name].browse(r['ids'])
self.assertEqual(rec.value_parent, 0)
self.assertEqual(rec.value, 1)
self.assertEqual(rec.parent_id.value_parent, 0)
self.assertEqual(
rec._get_external_ids()[rec.id],
['xxx.child'],
)
self.assertEqual(
rec.parent_id._get_external_ids()[rec.parent_id.id],
['xxx.child_export_inherits_parent'],
)
def test_create_parent_no_xid(self):
parent = self.env['export.inherits.parent'].create({'value_parent': 0})
r = self.import_(['id', 'parent_id/.id', 'value'], [
['xxx.child', str(parent.id), '1'],
])
rec = self.env[self.model_name].browse(r['ids'])
self.assertEqual(rec.value_parent, 0)
self.assertEqual(rec.parent_id, parent)
self.assertEqual(
rec._get_external_ids()[rec.id],
['xxx.child'],
)
self.assertEqual(
rec.parent_id._get_external_ids()[rec.parent_id.id],
[],
"no xid should be created for the parent"
)
def test_create_parent_with_xid(self):
parent = self.env['export.inherits.parent'].create({'value_parent': 0})
pid = self.env['ir.model.data'].create({
'model': 'export.inherits.parent',
'res_id': parent.id,
'module': 'xxx',
'name': 'parent',
})
r = self.import_(['id', 'parent_id/.id', 'value'], [
['xxx.child', str(parent.id), '1'],
])
rec = self.env[self.model_name].browse(r['ids'])
self.assertEqual(rec.value_parent, 0)
self.assertEqual(rec.parent_id, parent)
self.assertTrue(pid.exists().res_id, parent.id)
self.assertEqual(
rec._get_external_ids()[rec.id],
['xxx.child'],
)
self.assertEqual(
rec.parent_id._get_external_ids()[rec.parent_id.id],
['xxx.parent'],
)
def test_create_parent_by_xid(self):
parent = self.env['export.inherits.parent'].create({'value_parent': 0})
pid = self.env['ir.model.data'].create({
'model': 'export.inherits.parent',
'res_id': parent.id,
'module': 'xxx',
'name': 'parent',
})
r = self.import_(['id', 'parent_id/id', 'value'], [
['xxx.child', 'xxx.parent', '1'],
])
rec = self.env[self.model_name].browse(r['ids'])
self.assertEqual(rec.value_parent, 0)
self.assertEqual(rec.parent_id, parent)
self.assertTrue(pid.exists().res_id, parent.id)
self.assertEqual(
rec._get_external_ids()[rec.id],
['xxx.child'],
)
self.assertEqual(
rec.parent_id._get_external_ids()[rec.parent_id.id],
['xxx.parent'],
)
def test_update_parent_no_xid(self):
parent = self.env['export.inherits.parent'].create({'value_parent': 0})
child = self.env[self.model_name].create({
'parent_id': parent.id,
'value': 1,
})
self.env['ir.model.data'].create({
'model': self.model_name,
'res_id': child.id,
'module': 'xxx',
'name': 'child'
})
self.import_(['id', 'value'], [
['xxx.child', '42']
])
self.assertEqual(child.value, 42)
self.assertEqual(child.parent_id, parent)
self.assertEqual(
child._get_external_ids()[child.id],
['xxx.child'],
)
self.assertEqual(
parent._get_external_ids()[parent.id],
[],
)
def test_update_parent_with_xid(self):
parent = self.env['export.inherits.parent'].create({'value_parent': 0})
child = self.env[self.model_name].create({
'parent_id': parent.id,
'value': 1,
})
pid, cid = self.env['ir.model.data'].create([{
'model': 'export.inherits.parent',
'res_id': parent.id,
'module': 'xxx',
'name': 'parent',
}, {
'model': self.model_name,
'res_id': child.id,
'module': 'xxx',
'name': 'child'
}])
self.import_(['id', 'value'], [
['xxx.child', '42']
])
self.assertEqual(child.value, 42)
self.assertEqual(child.parent_id, parent)
self.assertEqual(pid.exists().res_id, parent.id)
self.assertEqual(cid.exists().res_id, child.id)
self.assertEqual(
child._get_external_ids()[child.id],
['xxx.child'],
)
self.assertEqual(
parent._get_external_ids()[parent.id],
['xxx.parent'],
)
odoo/models.py
+16 -22
View File
@@ -1047,7 +1047,7 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
for id, xid, record, info in converted:
if xid:
xid = xid if '.' in xid else "%s.%s" % (current_module, xid)
batch_xml_ids.update(ModelData._generate_xmlids(xid, self))
batch_xml_ids.add(xid)
elif id:
record['id'] = id
batch.append((xid, record, info))
@@ -4063,6 +4063,7 @@ Fields:
# determine which records to create and update
to_create = [] # list of data
to_update = [] # list of data
imd_data_list = [] # list of data for _update_xmlids()
for data in data_list:
xml_id = data.get('xml_id')
@@ -4080,11 +4081,11 @@ Fields:
continue
d_id, d_module, d_name, d_model, d_res_id, d_noupdate, r_id = row
record = self.browse(d_res_id)
if update and d_noupdate:
if r_id:
data['record'] = record
elif r_id:
data['record'] = record
to_update.append(data)
imd_data_list.append(data)
if not (update and d_noupdate):
to_update.append(data)
else:
imd.browse(d_id).unlink()
to_create.append(data)
@@ -4093,22 +4094,6 @@ Fields:
for data in to_update:
data['record']._load_records_write(data['values'])
# determine existing parents for new records
for parent_model, parent_field in self._inherits.items():
suffix = '_' + parent_model.replace('.', '_')
xml_ids_vals = {
(data['xml_id'] + suffix): data['values']
for data in to_create
if data.get('xml_id')
}
for row in imd._lookup_xmlids(xml_ids_vals, self.env[parent_model]):
d_id, d_module, d_name, d_model, d_res_id, d_noupdate, r_id = row
if r_id:
xml_id = '%s.%s' % (d_module, d_name)
xml_ids_vals[xml_id][parent_field] = r_id
else:
imd.browse(d_id).unlink()
# check for records to create with an XMLID from another module
module = self.env.context.get('install_module')
if module:
@@ -4121,9 +4106,18 @@ Fields:
records = self._load_records_create([data['values'] for data in to_create])
for data, record in zip(to_create, records):
data['record'] = record
if data.get('xml_id'):
# add XML ids for parent records that have just been created
for parent_model, parent_field in self._inherits.items():
if not data['values'].get(parent_field):
imd_data_list.append({
'xml_id': f"{data['xml_id']}_{parent_model.replace('.', '_')}",
'record': record[parent_field],
'noupdate': data.get('noupdate', False),
})
imd_data_list.append(data)
# create or update XMLIDs
imd_data_list = [data for data in data_list if data.get('xml_id')]
imd._update_xmlids(imd_data_list, update)
return original_self.concat(*(data['record'] for data in data_list))