[FIX] core: imports with one2many lines with dangling external ids

Consider a model M with a one2many field with comodel L, such that
deleting records in M cascade-deletes records in L.  Import a bunch of
records in M with one2many lines having specific external ids.  Delete
the imported records: the database cascade-deletes the corresponding
lines, but the lines' external ids are not deleted.  Now re-import the
same records: this crashes, because the import process finds the lines'
external ids and tries to update the deleted lines!

The fix consists in checking that an external id's record actually
exists.  The existence check was removed for performance reasons by
https://github.com/odoo/odoo/pull/26496.  We reintroduce it by resolving
the external id with a join on the model's name, which checks the
record's existence faster than an extra query.  This patch allows to
re-create the imported records' lines.

Another patch is necessary to actually update the external ids that
refer to deleted records, otherwise they remain dangling, and importing
the same records again duplicates the lines, because their external ids
do not match any existing record.

OPW 2409649

closes odoo/odoo#63318

X-original-commit: 0322febbea2cfd2d2fd7aedc42ef03632b17c967
Related: odoo/enterprise#15285
Signed-off-by: Raphael Collet (rco) <rco@openerp.com>
This commit is contained in:
Raphael Collet
2020-12-14 11:29:21 +00:00
parent 0684219d59
commit f2d4ba6890
3 changed files with 31 additions and 3 deletions
+26 -1
View File
@@ -337,7 +337,7 @@ class IrFieldsConverter(models.AbstractModel):
else:
xmlid = "%s.%s" % (self._context.get('_import_current_module', ''), value)
flush(xml_id=xmlid)
id = self.env['ir.model.data'].xmlid_to_res_id(xmlid, raise_if_not_found=False) or None
id = self._xmlid_to_record_id(xmlid, RelatedModel)
elif subfield is None:
field_type = _(u"name")
if value == '':
@@ -376,6 +376,31 @@ class IrFieldsConverter(models.AbstractModel):
{'moreinfo': action})
return id, field_type, warnings
def _xmlid_to_record_id(self, xmlid, model):
""" Return the record id corresponding to the given external id,
provided that the record actually exists; otherwise return ``None``.
"""
import_cache = self.env.context.get('import_cache', {})
result = import_cache.get(xmlid)
if not result:
module, name = xmlid.split('.', 1)
query = """
SELECT d.model, d.res_id
FROM ir_model_data d
JOIN "{}" r ON d.res_id = r.id
WHERE d.module = %s AND d.name = %s
""".format(model._table)
self.env.cr.execute(query, [module, name])
result = self.env.cr.fetchone()
if result:
res_model, res_id = import_cache[xmlid] = result
if res_model != model._name:
MSG = "Invalid external ID %s: expected model %r, found %r"
raise ValueError(MSG % (xmlid, model._name, res_model))
return res_id
def _referencing_subfield(self, record):
""" Checks the record for the subfields allowing referencing (an
existing record in an other table), errors out if it finds potential
+3 -1
View File
@@ -2010,7 +2010,9 @@ class IrModelData(models.Model):
INSERT INTO ir_model_data (module, name, model, res_id, noupdate)
VALUES {rows}
ON CONFLICT (module, name)
DO UPDATE SET write_date=(now() at time zone 'UTC') {where}
DO UPDATE SET (model, res_id, write_date) =
(EXCLUDED.model, EXCLUDED.res_id, now() at time zone 'UTC')
{where}
""".format(
rows=", ".join([rowf] * len(sub_rows)),
where="WHERE NOT ir_model_data.noupdate" if update else "",
+2 -1
View File
@@ -62,6 +62,7 @@ from .tools.misc import CountingStream, clean_context, DEFAULT_SERVER_DATETIME_F
from .tools.translate import _
from .tools import date_utils
from .tools import populate
from .tools.lru import LRU
_logger = logging.getLogger(__name__)
_schema = logging.getLogger(__name__ + '.schema')
@@ -1090,7 +1091,7 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
# make 'flush' available to the methods below, in the case where XMLID
# resolution fails, for instance
flush_self = self.with_context(import_flush=flush)
flush_self = self.with_context(import_flush=flush, import_cache=LRU(1024))
# TODO: break load's API instead of smuggling via context?
limit = self._context.get('_import_limit')