From 18f885b10fbf3c77cd833ca55bb53ac2ba83e988 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Tue, 6 Nov 2012 09:42:06 +0100 Subject: [PATCH] [FIX] recursive conversion of o2ms in import added new test cases from gkr bzr revid: xmo@openerp.com-20121106084206-1cckepflh9h4g1yv --- openerp/addons/base/ir/ir_fields.py | 68 ++++++++++++++++++- openerp/osv/orm.py | 35 +++------- openerp/tests/addons/test_impex/models.py | 9 +++ .../addons/test_impex/tests/contacts.json | 1 + .../addons/test_impex/tests/test_load.py | 47 ++++++++++++- 5 files changed, 129 insertions(+), 31 deletions(-) create mode 100644 openerp/tests/addons/test_impex/tests/contacts.json diff --git a/openerp/addons/base/ir/ir_fields.py b/openerp/addons/base/ir/ir_fields.py index 94a0be9e86c..105176bbd9c 100644 --- a/openerp/addons/base/ir/ir_fields.py +++ b/openerp/addons/base/ir/ir_fields.py @@ -1,4 +1,5 @@ # -*- coding: utf-8 -*- +import collections import datetime import functools import operator @@ -31,9 +32,70 @@ REPLACE_WITH = lambda ids: (6, False, ids) class ConversionNotFound(ValueError): pass +class ColumnWrapper(object): + def __init__(self, column, cr, uid, pool, fromtype, context=None): + self._converter = None + self._column = column + if column._obj: + self._pool = pool + self._converter_args = { + 'cr': cr, + 'uid': uid, + 'model': pool[column._obj], + 'fromtype': fromtype, + 'context': context + } + @property + def converter(self): + if not self._converter: + self._converter = self._pool['ir.fields.converter'].for_model( + **self._converter_args) + return self._converter + + def __getattr__(self, item): + return getattr(self._column, item) + class ir_fields_converter(orm.Model): _name = 'ir.fields.converter' + def for_model(self, cr, uid, model, fromtype=str, context=None): + """ Returns a converter object for the model. A converter is a + callable taking a record-ish (a dictionary representing an openerp + record with values of typetag ``fromtype``) and returning a converted + records matching what :meth:`openerp.osv.orm.Model.write` expects. + + :param model: :class:`openerp.osv.orm.Model` for the conversion base + :returns: a converter callable + :rtype: (record: dict, logger: (field, error) -> None) -> dict + """ + columns = dict( + (k, ColumnWrapper(v.column, cr, uid, self.pool, fromtype, context)) + for k, v in model._all_columns.iteritems()) + converters = dict( + (k, self.to_field(cr, uid, model, column, fromtype, context)) + for k, column in columns.iteritems()) + + def fn(record, log): + converted = {} + for field, value in record.iteritems(): + if field in (None, 'id', '.id'): continue + if not value: + converted[field] = False + continue + try: + converted[field], ws = converters[field](value) + for w in ws: + if isinstance(w, basestring): + # wrap warning string in an ImportWarning for + # uniform handling + w = ImportWarning(w) + log(field, w) + except ValueError, e: + log(field, e) + + return converted + return fn + def to_field(self, cr, uid, model, column, fromtype=str, context=None): """ Fetches a converter for the provided column object, from the specified type. @@ -343,6 +405,10 @@ class ir_fields_converter(orm.Model): # [{subfield:ref1},{subfield:ref2},{subfield:ref3}] records = ({subfield:item} for item in record[subfield].split(',')) + def log(_, e): + if not isinstance(e, Warning): + raise e + warnings.append(e) for record in records: id = None refs = only_ref_fields(record) @@ -355,7 +421,7 @@ class ir_fields_converter(orm.Model): cr, uid, model, column, subfield, reference, context=context) warnings.extend(w2) - writable = exclude_ref_fields(record) + writable = column.converter(exclude_ref_fields(record), log) if id: commands.append(LINK_TO(id)) commands.append(UPDATE(id, writable)) diff --git a/openerp/osv/orm.py b/openerp/osv/orm.py index a866b843582..910e08b3a69 100644 --- a/openerp/osv/orm.py +++ b/openerp/osv/orm.py @@ -1460,15 +1460,16 @@ class BaseModel(object): field_names = dict( (f, (Translation._get_source(cr, uid, self._name + ',' + f, 'field', context.get('lang')) - or column.string or f)) + or column.string)) for f, column in columns.iteritems()) - converters = dict( - (k, Converter.to_field(cr, uid, self, column, context=context)) - for k, column in columns.iteritems()) + + convert = Converter.for_model(cr, uid, self, context=context) def _log(base, field, exception): type = 'warning' if isinstance(exception, Warning) else 'error' - record = dict(base, field=field, type=type, + # logs the logical (not human-readable) field name for automated + # processing of response, but injects human readable in message + record = dict(base, type=type, field=field, message=unicode(exception.args[0]) % base) if len(exception.args) > 1 and exception.args[1]: record.update(exception.args[1]) @@ -1478,7 +1479,6 @@ class BaseModel(object): for record, extras in stream: dbid = False xid = False - converted = {} # name_get/name_create if None in record: pass # xid @@ -1499,27 +1499,8 @@ class BaseModel(object): message=_(u"Unknown database identifier '%s'") % dbid)) dbid = False - for field, strvalue in record.iteritems(): - if field in (None, 'id', '.id'): continue - if not strvalue: - converted[field] = False - continue - - # In warnings and error messages, use translated string as - # field name - message_base = dict( - extras, record=stream.index, field=field_names[field]) - try: - converted[field], ws = converters[field](strvalue) - - for w in ws: - if isinstance(w, basestring): - # wrap warning string in an ImportWarning for - # uniform handling - w = ImportWarning(w) - _log(message_base, field, w) - except ValueError, e: - _log(message_base, field, e) + converted = convert(record, lambda field, err:\ + _log(dict(extras, record=stream.index, field=field_names[field]), field, err)) yield dbid, xid, converted, dict(extras, record=stream.index) diff --git a/openerp/tests/addons/test_impex/models.py b/openerp/tests/addons/test_impex/models.py index 48bd9948a0f..95a7f90bda8 100644 --- a/openerp/tests/addons/test_impex/models.py +++ b/openerp/tests/addons/test_impex/models.py @@ -79,6 +79,7 @@ class One2ManyMultiple(orm.Model): _name = 'export.one2many.multiple' _columns = { + 'parent_id': fields.many2one('export.one2many.recursive'), 'const': fields.integer(), 'child1': fields.one2many('export.one2many.child.1', 'parent_id'), 'child2': fields.one2many('export.one2many.child.2', 'parent_id'), @@ -135,3 +136,11 @@ class SelectionWithDefault(orm.Model): 'const': 4, 'value': 2, } + +class RecO2M(orm.Model): + _name = 'export.one2many.recursive' + + _columns = { + 'value': fields.integer(), + 'child': fields.one2many('export.one2many.multiple', 'parent_id') + } diff --git a/openerp/tests/addons/test_impex/tests/contacts.json b/openerp/tests/addons/test_impex/tests/contacts.json new file mode 100644 index 00000000000..f9f185c141f --- /dev/null +++ b/openerp/tests/addons/test_impex/tests/contacts.json @@ -0,0 +1 @@ +[["Wood y Wood Pecker", "", "Snow Street, 25", "Kainuu", "Finland", "Supplier", "1", "0", "1", ""], ["Roger Pecker", "Default", "Snow Street, 27", "Kainuu", "Finland", "Supplier", "1", "0", "0", "Wood y Wood Pecker"], ["Sharon Pecker", "Shipping", "Snow Street, 28", "Kainuu", "Finland", "Supplier", "1", "0", "0", "Wood y Wood Pecker"], ["Thomas Pecker", "Contact", "Snow Street, 27", "Kainuu", "Finland", "Supplier", "1", "0", "0", "Wood y Wood Pecker"], ["Vicking Direct", "", "Atonium Street, 45a", "Brussels", "Belgium", "Supplier", "1", "0", "1", ""], ["Yvan Holiday", "Invoice", "Atonium Street, 45b", "Brussels", "Belgium", "Supplier", "1", "0", "0", "Vicking Direct"], ["Jack Unsworth", "Contact", "Atonium Street, 45a", "Brussels", "Belgium", "Supplier", "1", "0", "0", "Vicking Direct"]] diff --git a/openerp/tests/addons/test_impex/tests/test_load.py b/openerp/tests/addons/test_impex/tests/test_load.py index 8dac916b0d0..9ef2a43dec9 100644 --- a/openerp/tests/addons/test_impex/tests/test_load.py +++ b/openerp/tests/addons/test_impex/tests/test_load.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- import json import pkgutil +import unittest2 import openerp.modules.registry import openerp @@ -246,7 +247,7 @@ class test_integer_field(ImporterCase): -1, -42, -(2**31 - 1), -(2**31), -12345678 ], values(self.read())) - @mute_logger('openerp.sql_db') + @mute_logger('openerp.sql_db', 'openerp.osv.orm') def test_out_of_range(self): result = self.import_(['value'], [[str(2**31)]]) self.assertIs(result['ids'], False) @@ -388,14 +389,14 @@ class test_unbound_string_field(ImporterCase): class test_required_string_field(ImporterCase): model_name = 'export.string.required' - @mute_logger('openerp.sql_db') + @mute_logger('openerp.sql_db', 'openerp.osv.orm') def test_empty(self): result = self.import_(['value'], [[]]) self.assertEqual(result['messages'], [message( u"Missing required value for the field 'unknown'")]) self.assertIs(result['ids'], False) - @mute_logger('openerp.sql_db') + @mute_logger('openerp.sql_db', 'openerp.osv.orm') def test_not_provided(self): result = self.import_(['const'], [['12']]) self.assertEqual(result['messages'], [message( @@ -1007,6 +1008,46 @@ class test_realworld(common.TransactionCase): self.assertFalse(result['messages']) self.assertEqual(len(result['ids']), len(data)) + def test_backlink(self): + data = json.loads(pkgutil.get_data(self.__module__, 'contacts.json')) + result = self.registry('res.partner').load( + self.cr, openerp.SUPERUSER_ID, + ["name", "type", "street", "city", "country_id", "category_id", + "supplier", "customer", "is_company", "parent_id"], + data) + self.assertFalse(result['messages']) + self.assertEqual(len(result['ids']), len(data)) + + def test_recursive_o2m(self): + """ The content of the o2m field's dict needs to go through conversion + as it may be composed of convertables or other relational fields + """ + self.registry('ir.model.data').clear_caches() + Model = self.registry('export.one2many.recursive') + result = Model.load(self.cr, openerp.SUPERUSER_ID, + ['value', 'child/const', 'child/child1/str', 'child/child2/value'], + [ + ['4', '42', 'foo', '55'], + ['', '43', 'bar', '56'], + ['', '', 'baz', ''], + ['', '55', 'qux', '57'], + ['5', '99', 'wheee', ''], + ['', '98', '', '12'], + ], + context=None) + + self.assertFalse(result['messages']) + self.assertEqual(len(result['ids']), 2) + + b = Model.browse(self.cr, openerp.SUPERUSER_ID, result['ids'], context=None) + self.assertEqual((b[0].value, b[1].value), (4, 5)) + + self.assertEqual([child.str for child in b[0].child[1].child1], + ['bar', 'baz']) + self.assertFalse(len(b[1].child[1].child1)) + self.assertEqual([child.value for child in b[1].child[1].child2], + [12]) + class test_date(ImporterCase): model_name = 'export.date'