[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

@@ -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))

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)
 

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')
+    }

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"]]

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'