diff --git a/trytond/CHANGELOG b/trytond/CHANGELOG index 35c4673465..026ea959f9 100644 --- a/trytond/CHANGELOG +++ b/trytond/CHANGELOG @@ -1,3 +1,4 @@ +* Warn about implicit save() of unsaved relation * Raise error message with record name and field in check_xml_record * Cache RPC call for class method selection diff --git a/trytond/trytond/model/modelstorage.py b/trytond/trytond/model/modelstorage.py index 419c4fc4f4..f90bcd62d9 100644 --- a/trytond/trytond/model/modelstorage.py +++ b/trytond/trytond/model/modelstorage.py @@ -7,6 +7,7 @@ import datetime import decimal import random import time +import warnings from collections import defaultdict from decimal import Decimal from functools import lru_cache, wraps @@ -84,6 +85,12 @@ class TimeFormatValidationError(ValidationError): pass +class _UnsavedRecordError(ValueError): + + def __init__(self, record): + self.record = record + + def without_check_access(func): @wraps(func) def wrapper(*args, **kwargs): @@ -1833,7 +1840,7 @@ class ModelStorage(Model): if value: if ((value.id is None or value.id < 0) and field._type != 'reference'): - value.save() + raise _UnsavedRecordError(value) if field._type == 'reference': value = str(value) else: @@ -1924,7 +1931,14 @@ class ModelStorage(Model): or context != record._context): latter.append(record) continue - save_values[record] = record._save_values + while True: + try: + save_values[record] = record._save_values + break + except _UnsavedRecordError as e: + warnings.warn( + f"Using an unsaved relation record: {e.record}") + e.record.save() values[record] = record._values record._values = None if record.id is None or record.id < 0: diff --git a/trytond/trytond/tests/modelstorage.py b/trytond/trytond/tests/modelstorage.py index 4e3fa6c967..a3c5364f7d 100644 --- a/trytond/trytond/tests/modelstorage.py +++ b/trytond/trytond/tests/modelstorage.py @@ -20,6 +20,28 @@ class ModelStorageRequired(ModelSQL): name = fields.Char('Name', required=True) +class ModelStorageSaveMany2One(ModelSQL): + "ModelStorage to test save with Many2One" + __name__ = 'test.modelstorage.save_m2o' + target = fields.Many2One('test.modelstorage.save_m2o.target', "Target") + targets = fields.One2Many( + 'test.modelstorage.save_m2o.target', 'parent', "Targets") + + +class ModelStorageSaveMany2OneTarget(ModelSQL): + "ModelStorage to test save with Many2One - Target" + __name__ = 'test.modelstorage.save_m2o.target' + + parent = fields.Many2One('test.modelstorage.save_m2o', "Parent") + target = fields.Many2One( + 'test.modelstorage.save_m2o.o2m_target.target', "Target") + + +class ModelStorageSaveMany2OneTargetTarget(ModelSQL): + "ModelStorage to test save with Many2One - Target of Target" + __name__ = 'test.modelstorage.save_m2o.o2m_target.target' + + class ModelStorageSave2Many(ModelSQL): "Model Storage to test save with xxx2many" __name__ = 'test.modelstorage.save2many' @@ -176,6 +198,9 @@ def register(module): Pool.register( ModelStorage, ModelStorageRequired, + ModelStorageSaveMany2One, + ModelStorageSaveMany2OneTarget, + ModelStorageSaveMany2OneTargetTarget, ModelStorageSave2Many, ModelStorageSave2ManyTarget, ModelStorageSave2ManyRelation, diff --git a/trytond/trytond/tests/test_modelstorage.py b/trytond/trytond/tests/test_modelstorage.py index 1714ef5174..90c4159283 100644 --- a/trytond/trytond/tests/test_modelstorage.py +++ b/trytond/trytond/tests/test_modelstorage.py @@ -6,6 +6,7 @@ import unittest from trytond.model import EvalEnvironment from trytond.model.exceptions import ( AccessError, DomainValidationError, RequiredValidationError) +from trytond.model.modelstorage import _UnsavedRecordError from trytond.pool import Pool from trytond.tests.test_tryton import activate_module, with_transaction from trytond.transaction import Transaction @@ -151,6 +152,78 @@ class ModelStorageTestCase(unittest.TestCase): with self.assertRaises(RequiredValidationError): ModelStorage.save([foo, bar]) + @with_transaction() + def test_many2one_save_raise_exception(self): + "Test _save_values raises _UnsavedRecordError on unsaved record" + pool = Pool() + ModelStorage = pool.get('test.modelstorage.save_m2o') + Target = pool.get('test.modelstorage.save_m2o.target') + + record = ModelStorage() + record.target = target = Target() + + with self.assertRaises(_UnsavedRecordError) as cm: + record._save_values + + self.assertEqual(cm.exception.record, target) + + @with_transaction() + def test_save_many2one_create(self): + "Test saving a record while creating a Many2One" + pool = Pool() + ModelStorage = pool.get('test.modelstorage.save_m2o') + Target = pool.get('test.modelstorage.save_m2o.target') + + record = ModelStorage() + record.target = target = Target() + with self.assertWarnsRegex( + UserWarning, r"Using an unsaved relation record:"): + record.save() + + self.assertIsNotNone(record.id) + self.assertIsNotNone(target.id) + + @with_transaction() + def test_save_many2one_create_multi(self): + "Test saving multiple records while creating a M2O" + pool = Pool() + ModelStorage = pool.get('test.modelstorage.save_m2o') + Target = pool.get('test.modelstorage.save_m2o.target') + + record1 = ModelStorage() + record1.target = target = Target() + record1.save() + + record2 = ModelStorage() + record2.target = target + record2.save() + + ModelStorage.save([record1, record2]) + + self.assertIsNotNone(record1.id) + self.assertIsNotNone(record2.id) + self.assertEqual(record1.target.id, record2.target.id) + self.assertNotEqual(record2.id, record1.id) + + @with_transaction() + def test_save_many2one_in_one2many_create(self): + "Test saving a record while creating a M2O contained in a O2M" + pool = Pool() + ModelStorage = pool.get('test.modelstorage.save_m2o') + Target = pool.get('test.modelstorage.save_m2o.target') + M2OTarget = pool.get('test.modelstorage.save_m2o.o2m_target.target') + + target = Target() + target.target = m2o_target = M2OTarget() + record = ModelStorage() + record.targets = [target] + record.save() + + self.assertIsNotNone(record.id) + self.assertIsNotNone(m2o_target.id) + self.assertEqual(len(record.targets), 1) + self.assertEqual(record.targets[0].target, m2o_target) + @with_transaction() def test_save_one2many_create(self): "Test save one2many create"