Merge lp:~openerp-connector-core-editors/openerp-connector/7.0-connector-mapper-refactor into lp:~openerp-connector-core-editors/openerp-connector/7.0

[Guewen Baconnier @ Camptocamp]

Refactoring of the Mappers.
===========================

Work in progress.

Now that I have gained more experience with my framework and that I have been confronted with different implementations, I discover some potentialities as well as some of its limitations.

This proposal aims to address some issues and limitations of the actual mappers.

It is _as possible_ backward compatible, but a few things will need to be changed in the implementations using the connector (not in the Mappers themselves though, but in the way we use them, so the change has a limited effect).

Rationale
---------

State
~~~~~

The synchronizers have a a direct access to the most used ConnectorUnit classes, self.binder, self.backend_adapter, self.mapper will return an instance of the appropriate one. The same instance is reused. There is a design issue on the Mappers because they contains a state of the records:

    self.mapper.convert(record)
    values = self.mapper.data

One of my point was to re-establish a stateless Mapper. Now, the mapper returns a MapRecord prepared for the mapping. The MapRecord can ask to be mapped at any moment:

    map_record = self.mapper.map_record(record)
    map_record2 = self.mapper.map_record(record2)
    values = map_record.values()

(in fact, there is some transient state in the Mapper, only when we call map_record.values(), which is self.options, so it is not thread-safe - that is not a goal -, but there is no issues like before).

This forms allows brings a nice feature which are the options.

Options
~~~~~~~

The new form with a MapRecord allows to specify custom options for the mappings, example:

      map_record = self.mapper.map_record(record)
      values = map_record.values(tax_include=True)

      # in the mapper
      @mapping
      def fees(self, record):
          if self.options.tax_include:
              return ...
          else:
              return ...

The 'for_create' and 'fields' are now handled like options.

Coupling of "children" mappings
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Children mappings are, for instance, the lines of a sales order that we want to create in the same call to "create()" than the sales order's one.

In the current code, they are defined in the mappers with:

   children = [('items', 'magento_order_line_ids', 'magento.sale.order.line')]

Where 'items' is the key containing the lines in the source record, 'magento_order_line_ids' the key in the target record and 'magento.sale.order.line' the name of the model of the lines.
The connector would search the Mapper for the model of the lines, process each lines and add them in the target record (for instance with the OpenERP commands (0, 0, {values}).

This was done directly in the main Mapper and thus was completely coupled with it. It brought difficulties when we wanted a more elaborate behavior than the default one: skip some lines, update existing lines instead of just creating them...

So I introduce a "MapChild" ConnectorUnit, which defines *how* items are converted and added to the main target record. The result of the default implementation is similar to the previous one. It can be easily extended to customize it though. It is not mandatory to create a MapChild each time we use 'children', the default one will be used if it not defined (although this ensures the backward compatibility).

Changes after the mapping
~~~~~~~~~~~~~~~~~~~~~~~~~

The Mappers apply all the 'direct' mappings, then all the @mapping methods, then all the 'children' mappings. Finally, we have the opportunity to override _after_mapping() so we can alter the values just before they are returned by the Mapper. Unfortunately, this method wasn't aware of the source values, which was a pity since we may need them if we want to change the target values. This is fixed in the new finalize() method.

Modifiers
~~~~~~~~~

See https://code.launchpad.net/~camptocamp/openerp-connector/7.0-connector-closure-functions/+merge/193389

Non-Backward compatible changes
-------------------------------

Usage of a Mapper
~~~~~~~~~~~~~~~~~
Previously:

    self.mapper.convert(record, fields=['a', 'b')
    values = self.mapper.data
    values = self.mapper.data_for_create

Now:

    map_record = self.mapper.map_record(record)
    values = map_record.values()
    values = map_record.values(for_create=True)
    values = map_record.values(fields=['a', 'b')
    values = map_record.values(tax_include=True)

This change impacts the Synchronizers.

Filtering of items
~~~~~~~~~~~~~~~~~~

For 'children' mappings (submappings), (e.g. lines of a sales order),
the way in place to filter out some items was:

       # method in 'Mapper'
       def skip_convert_child(self, record, parent_values=None):
        return record['type'] == 'configurable'

Now, we need to create a 'MapChild' ConnectorUnit that will decide *how* the items are converted (note that creating a 'MapChild' is not necessary as soon as we don't need to modify its default behavior):

    @magento
    class SalesLineMapChild(MapChild):
        _model_name = 'magento.sale.order.line'

        def skip_item(self, map_record):
        # map_record.parent is accessible
        return map_record.source['type'] == 'configurable'

Changes at the end of a mapping
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

At the end of a mapping, Mapper._after_mapping() was called with the output values, letting the developer changing the values when all of them are known. This was handy for applying onchanges for instance. Though, it was not aware of the source values,
which was unfortunate if we needed them.

Now, instead of overriding Mapper._after_mapping(self, record), override:

    def finalize(self, map_record, values):
        # call onchanges...
        # add extra order lines...
        return values

Concretely
~~~~~~~~~~

Here are the changes I needed to do for the Magento Connector:
https://code.launchpad.net/~camptocamp/openerp-connector-magento/7.0-new-mapper-api-gbr/+merge/194630

----------------------------------------------------------------------
Ran 74 tests in 4.278s

OK

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Branch is now sufficiently stable to be tested and used for development.

[Joël Grand-Guillaume @ camptocamp (jgrandguillaume-c2c)]

Hi,

Thanks for this work ! As far as my skills can judge of that MP, it LGTM. Tested on my machine, all tests are green and I found no coding standard issues so I approve this proposal.

Though, I'll feel comfortable here if a more invested person could validate that work.

Regards,

Joël

review: Approve (code review, no tests)

[lp:~openerp-connector-core-editors/openerp-connector/7.0-connector-mapper-refactor]

594. By Guewen Baconnier @ Camptocamp on 2013-12-20

[IMP] extract the build of the MapChild unit instance to allow better reusability

595. By Guewen Baconnier @ Camptocamp on 2013-12-24

[MRG] from lp:openerp-connector

=== modified file 'connector/CHANGES.rst'
--- connector/CHANGES.rst	2013-12-24 07:45:03 +0000
+++ connector/CHANGES.rst	2013-12-24 07:45:03 +0000
@@ -4,17 +4,13 @@
 2.0.1.dev0
 ~~~~~~~~~~
 
-<<<<<<< TREE
 * Add a new optional keyword argument 'description' to the delay() function of a
   job.  If given, the description is used as name of the queue.job record stored
   in OpenERP and displayed in the list of jobs.
-* Fix: assignment of jobs to workers respect the priority of the jobs (lp:1252681)
-
-=======
 * Add the possibility to use 'Modifiers' functions in the 'direct
   mappings' (details in the documentation of the Mapper class)
+* Fix: assignment of jobs to workers respect the priority of the jobs (lp:1252681)
 
->>>>>>> MERGE-SOURCE
 2.0.1 (2013-09-12)
 ~~~~~~~~~~~~~~~~~~
 

=== modified file 'connector/backend.py'
--- connector/backend.py	2013-08-26 20:05:18 +0000
+++ connector/backend.py	2013-12-24 07:45:03 +0000
@@ -20,6 +20,7 @@
 ##############################################################################
 from functools import partial
 from collections import namedtuple
+from .exception import NoConnectorUnitError
 
 __all__ = ['Backend']
 
@@ -257,10 +258,12 @@
         """
         matching_classes = self._get_classes(base_class, session,
                                              model_name)
-        assert matching_classes, ('No matching class found for %s '
-                                  'with session: %s, '
-                                  'model name: %s' %
-                                  (base_class, session, model_name))
+        if not matching_classes:
+            raise NoConnectorUnitError('No matching class found for %s '
+                                       'with session: %s, '
+                                       'model name: %s' %
+                                       (base_class, session, model_name))
+
         assert len(matching_classes) == 1, (
             'Several classes found for %s '
             'with session %s, model name: %s. Found: %s' %

=== modified file 'connector/exception.py'
--- connector/exception.py	2013-06-19 14:47:17 +0000
+++ connector/exception.py	2013-12-24 07:45:03 +0000
@@ -24,6 +24,10 @@
     """ Base Exception for the connectors """
 
 
+class NoConnectorUnitError(ConnectorException):
+    """ No ConnectorUnit has been found """
+
+
 class InvalidDataError(ConnectorException):
     """ Data Invalid """
 

=== modified file 'connector/tests/test_backend.py'
--- connector/tests/test_backend.py	2013-12-24 07:45:03 +0000
+++ connector/tests/test_backend.py	2013-12-24 07:45:03 +0000
@@ -6,6 +6,7 @@
 from openerp.addons.connector.backend import (Backend,
                                               get_backend,
                                               BACKENDS)
+from openerp.addons.connector.exception import NoConnectorUnitError
 from openerp.addons.connector.connector import (Binder,
                                                 ConnectorUnit)
 from openerp.addons.connector.unit.mapper import (Mapper,
@@ -110,7 +111,7 @@
 
     def test_no_register_error(self):
         """ Error when asking for a class and none is found"""
-        with self.assertRaises(AssertionError):
+        with self.assertRaises(NoConnectorUnitError):
             ref = self.backend.get_class(BackendAdapter,
                                          self.session,
                                          'res.users')

=== modified file 'connector/tests/test_mapper.py'
--- connector/tests/test_mapper.py	2013-12-24 07:45:03 +0000
+++ connector/tests/test_mapper.py	2013-12-24 07:45:03 +0000
@@ -7,6 +7,7 @@
 from openerp.addons.connector.unit.mapper import (
     Mapper,
     ImportMapper,
+    ImportMapChild,
     MappingDefinition,
     changed_by,
     only_create,
@@ -14,8 +15,10 @@
     m2o_to_backend,
     backend_to_m2o,
     none,
+    MapOptions,
     mapping)
 
+from openerp.addons.connector.exception import NoConnectorUnitError
 from openerp.addons.connector.backend import Backend
 from openerp.addons.connector.connector import Environment
 from openerp.addons.connector.session import ConnectorSession
@@ -160,11 +163,11 @@
         record = {'name': 'Guewen',
                     'street': 'street'}
         mapper = MyMapper(env)
-        mapper.convert(record)
+        map_record = mapper.map_record(record)
         expected = {'out_name': 'Guewen',
                     'out_street': 'STREET'}
-        self.assertEqual(mapper.data, expected)
-        self.assertEqual(mapper.data_for_create, expected)
+        self.assertEqual(map_record.values(), expected)
+        self.assertEqual(map_record.values(for_create=True), expected)
 
     def test_mapping_record_on_create(self):
         """ Map a record and check the result for creation of record """
@@ -185,64 +188,98 @@
         record = {'name': 'Guewen',
                   'street': 'street'}
         mapper = MyMapper(env)
-        mapper.convert(record)
+        map_record = mapper.map_record(record)
         expected = {'out_name': 'Guewen',
                     'out_street': 'STREET'}
-        self.assertEqual(mapper.data, expected)
+        self.assertEqual(map_record.values(), expected)
         expected = {'out_name': 'Guewen',
                     'out_street': 'STREET',
                     'out_city': 'city'}
-        self.assertEqual(mapper.data_for_create, expected)
-
-    def test_mapping_record_children(self):
-        """ Map a record with children and check the result """
-
-        class LineMapper(ImportMapper):
-            direct = [('name', 'name')]
+        self.assertEqual(map_record.values(for_create=True), expected)
+
+    def test_mapping_update(self):
+        """ Force values on a map record """
+        class MyMapper(ImportMapper):
+
+            direct = [('name', 'out_name')]
 
             @mapping
-            def price(self, record):
-                return {'price': record['price'] * 2}
+            def street(self, record):
+                return {'out_street': record['street'].upper()}
 
             @only_create
             @mapping
-            def discount(self, record):
-                return {'discount': .5}
-
-        class SaleMapper(ImportMapper):
-
-            direct = [('name', 'name')]
-
-            children = [('lines', 'line_ids', 'sale.order.line')]
-
-            def _init_child_mapper(self, model_name):
-                return LineMapper(self.environment)
-
-        env = mock.MagicMock()
-
-        record = {'name': 'SO1',
-                  'lines': [{'name': 'Mango',
-                             'price': 10},
-                            {'name': 'Pumpkin',
-                             'price': 20}]}
-        mapper = SaleMapper(env)
-        mapper.convert(record)
-        expected = {'name': 'SO1',
-                    'line_ids': [(0, 0, {'name': 'Mango',
-                                         'price': 20}),
-                                 (0, 0, {'name': 'Pumpkin',
-                                         'price': 40})]
-                    }
-        self.assertEqual(mapper.data, expected)
-        expected = {'name': 'SO1',
-                    'line_ids': [(0, 0, {'name': 'Mango',
-                                         'price': 20,
-                                         'discount': .5}),
-                                 (0, 0, {'name': 'Pumpkin',
-                                         'price': 40,
-                                         'discount': .5})]
-                    }
-        self.assertEqual(mapper.data_for_create, expected)
+            def city(self, record):
+                return {'out_city': 'city'}
+
+        env = mock.MagicMock()
+        record = {'name': 'Guewen',
+                  'street': 'street'}
+        mapper = MyMapper(env)
+        map_record = mapper.map_record(record)
+        map_record.update({'test': 1}, out_city='forced')
+        expected = {'out_name': 'Guewen',
+                    'out_street': 'STREET',
+                    'out_city': 'forced',
+                    'test': 1}
+        self.assertEqual(map_record.values(), expected)
+        expected = {'out_name': 'Guewen',
+                    'out_street': 'STREET',
+                    'out_city': 'forced',
+                    'test': 1}
+        self.assertEqual(map_record.values(for_create=True), expected)
+
+    def test_finalize(self):
+        """ Inherit finalize to modify values """
+        class MyMapper(ImportMapper):
+
+            direct = [('name', 'out_name')]
+
+            def finalize(self, record, values):
+                result = super(MyMapper, self).finalize(record, values)
+                result['test'] = 'abc'
+                return result
+
+        env = mock.MagicMock()
+        record = {'name': 'Guewen',
+                  'street': 'street'}
+        mapper = MyMapper(env)
+        map_record = mapper.map_record(record)
+        expected = {'out_name': 'Guewen',
+                    'test': 'abc'}
+        self.assertEqual(map_record.values(), expected)
+        expected = {'out_name': 'Guewen',
+                    'test': 'abc'}
+        self.assertEqual(map_record.values(for_create=True), expected)
+
+    def test_some_fields(self):
+        """ Map only a selection of fields """
+        class MyMapper(ImportMapper):
+
+            direct = [('name', 'out_name'),
+                      ('street', 'out_street'),
+                      ]
+
+            @changed_by('country')
+            @mapping
+            def country(self, record):
+                return {'country': 'country'}
+
+        env = mock.MagicMock()
+        record = {'name': 'Guewen',
+                  'street': 'street',
+                  'country': 'country'}
+        mapper = MyMapper(env)
+        map_record = mapper.map_record(record)
+        expected = {'out_name': 'Guewen',
+                    'country': 'country'}
+        self.assertEqual(map_record.values(fields=['name', 'country']),
+                         expected)
+        expected = {'out_name': 'Guewen',
+                    'country': 'country'}
+        self.assertEqual(map_record.values(for_create=True,
+                                           fields=['name', 'country']),
+                         expected)
 
     def test_mapping_modifier(self):
         """ Map a direct record with a modifier function """
@@ -258,10 +295,10 @@
         env = mock.MagicMock()
         record = {'name': 'Guewen'}
         mapper = MyMapper(env)
-        mapper.convert(record)
+        map_record = mapper.map_record(record)
         expected = {'out_name': 'Guewen'}
-        self.assertEqual(mapper.data, expected)
-        self.assertEqual(mapper.data_for_create, expected)
+        self.assertEqual(map_record.values(), expected)
+        self.assertEqual(map_record.values(for_create=True), expected)
 
     def test_mapping_convert(self):
         """ Map a direct record with the convert modifier function """
@@ -271,10 +308,10 @@
         env = mock.MagicMock()
         record = {'name': '300'}
         mapper = MyMapper(env)
-        mapper.convert(record)
+        map_record = mapper.map_record(record)
         expected = {'out_name': 300}
-        self.assertEqual(mapper.data, expected)
-        self.assertEqual(mapper.data_for_create, expected)
+        self.assertEqual(map_record.values(), expected)
+        self.assertEqual(map_record.values(for_create=True), expected)
 
     def test_mapping_modifier_none(self):
         """ Pipeline of modifiers """
@@ -285,10 +322,10 @@
         env = mock.MagicMock()
         record = {'in_f': False, 'in_t': True}
         mapper = MyMapper(env)
-        mapper.convert(record)
+        map_record = mapper.map_record(record)
         expected = {'out_f': None, 'out_t': True}
-        self.assertEqual(mapper.data, expected)
-        self.assertEqual(mapper.data_for_create, expected)
+        self.assertEqual(map_record.values(), expected)
+        self.assertEqual(map_record.values(for_create=True), expected)
 
     def test_mapping_modifier_pipeline(self):
         """ Pipeline of modifiers """
@@ -299,10 +336,59 @@
         env = mock.MagicMock()
         record = {'in_f': 0, 'in_t': 1}
         mapper = MyMapper(env)
-        mapper.convert(record)
+        map_record = mapper.map_record(record)
         expected = {'out_f': None, 'out_t': True}
-        self.assertEqual(mapper.data, expected)
-        self.assertEqual(mapper.data_for_create, expected)
+        self.assertEqual(map_record.values(), expected)
+        self.assertEqual(map_record.values(for_create=True), expected)
+
+    def test_mapping_custom_option(self):
+        """ Usage of custom options in mappings """
+        class MyMapper(ImportMapper):
+            @mapping
+            def any(self, record):
+                if self.options.custom:
+                    res = True
+                else:
+                    res = False
+                return {'res': res}
+
+        env = mock.MagicMock()
+        record = {}
+        mapper = MyMapper(env)
+        map_record = mapper.map_record(record)
+        expected = {'res': True}
+        self.assertEqual(map_record.values(custom=True), expected)
+
+    def test_mapping_custom_option_not_defined(self):
+        """ Usage of custom options not defined raise AttributeError """
+        class MyMapper(ImportMapper):
+            @mapping
+            def any(self, record):
+                if self.options.custom is None:
+                    res = True
+                else:
+                    res = False
+                return {'res': res}
+
+        env = mock.MagicMock()
+        record = {}
+        mapper = MyMapper(env)
+        map_record = mapper.map_record(record)
+        expected = {'res': True}
+        self.assertEqual(map_record.values(), expected)
+
+    def test_map_options(self):
+        """ Test MapOptions """
+        options = MapOptions({'xyz': 'abc'}, k=1)
+        options.l = 2
+        self.assertEqual(options['xyz'], 'abc')
+        self.assertEqual(options['k'], 1)
+        self.assertEqual(options['l'], 2)
+        self.assertEqual(options.xyz, 'abc')
+        self.assertEqual(options.k, 1)
+        self.assertEqual(options.l, 2)
+        self.assertEqual(options['undefined'], None)
+        self.assertEqual(options.undefined, None)
 
 
 class test_mapper_binding(common.TransactionCase):
@@ -334,10 +420,10 @@
         self.country_binder.to_backend.return_value = 10
 
         mapper = MyMapper(self.env)
-        mapper.convert(partner)
+        map_record = mapper.map_record(partner)
+        self.assertEqual(map_record.values(), {'country': 10})
         self.country_binder.to_backend.assert_called_once_with(
             partner.country_id.id, wrap=False)
-        self.assertEqual(mapper.data, {'country': 10})
 
     def test_mapping_backend_to_m2o(self):
         """ Map a direct record with the backend_to_m2o modifier function """
@@ -348,7 +434,127 @@
         record = {'country': 10}
         self.country_binder.to_openerp.return_value = 44
         mapper = MyMapper(self.env)
-        mapper.convert(record)
+        map_record = mapper.map_record(record)
+        self.assertEqual(map_record.values(), {'country_id': 44})
         self.country_binder.to_openerp.assert_called_once_with(
             10, unwrap=False)
-        self.assertEqual(mapper.data, {'country_id': 44})
+
+    def test_mapping_record_children_no_map_child(self):
+        """ Map a record with children, using default MapChild """
+
+        backend = Backend('backend', '42')
+
+        @backend
+        class LineMapper(ImportMapper):
+            _model_name = 'res.currency.rate'
+            direct = [('name', 'name')]
+
+            @mapping
+            def price(self, record):
+                return {'rate': record['rate'] * 2}
+
+            @only_create
+            @mapping
+            def discount(self, record):
+                return {'test': .5}
+
+        @backend
+        class ObjectMapper(ImportMapper):
+            _model_name = 'res.currency'
+
+            direct = [('name', 'name')]
+
+            children = [('lines', 'line_ids', 'res.currency.rate')]
+
+
+        backend_record = mock.Mock()
+        backend_record.get_backend.side_effect = lambda *a: backend
+        env = Environment(backend_record, self.session, 'res.currency')
+
+        record = {'name': 'SO1',
+                  'lines': [{'name': '2013-11-07',
+                             'rate': 10},
+                            {'name': '2013-11-08',
+                             'rate': 20}]}
+        mapper = ObjectMapper(env)
+        map_record = mapper.map_record(record)
+        expected = {'name': 'SO1',
+                    'line_ids': [(0, 0, {'name': '2013-11-07',
+                                         'rate': 20}),
+                                 (0, 0, {'name': '2013-11-08',
+                                         'rate': 40})]
+                    }
+        self.assertEqual(map_record.values(), expected)
+        expected = {'name': 'SO1',
+                    'line_ids': [(0, 0, {'name': '2013-11-07',
+                                         'rate': 20,
+                                         'test': .5}),
+                                 (0, 0, {'name': '2013-11-08',
+                                         'rate': 40,
+                                         'test': .5})]
+                    }
+        self.assertEqual(map_record.values(for_create=True), expected)
+
+    def test_mapping_record_children(self):
+        """ Map a record with children, using defined MapChild """
+
+        backend = Backend('backend', '42')
+
+        @backend
+        class LineMapper(ImportMapper):
+            _model_name = 'res.currency.rate'
+            direct = [('name', 'name')]
+
+            @mapping
+            def price(self, record):
+                return {'rate': record['rate'] * 2}
+
+            @only_create
+            @mapping
+            def discount(self, record):
+                return {'test': .5}
+
+        @backend
+        class SaleLineImportMapChild(ImportMapChild):
+            _model_name = 'res.currency.rate'
+
+            def format_items(self, items_values):
+                return [('ABC', values) for values in items_values]
+
+
+        @backend
+        class ObjectMapper(ImportMapper):
+            _model_name = 'res.currency'
+
+            direct = [('name', 'name')]
+
+            children = [('lines', 'line_ids', 'res.currency.rate')]
+
+
+        backend_record = mock.Mock()
+        backend_record.get_backend.side_effect = lambda *a: backend
+        env = Environment(backend_record, self.session, 'res.currency')
+
+        record = {'name': 'SO1',
+                  'lines': [{'name': '2013-11-07',
+                             'rate': 10},
+                            {'name': '2013-11-08',
+                             'rate': 20}]}
+        mapper = ObjectMapper(env)
+        map_record = mapper.map_record(record)
+        expected = {'name': 'SO1',
+                    'line_ids': [('ABC', {'name': '2013-11-07',
+                                          'rate': 20}),
+                                 ('ABC', {'name': '2013-11-08',
+                                          'rate': 40})]
+                    }
+        self.assertEqual(map_record.values(), expected)
+        expected = {'name': 'SO1',
+                    'line_ids': [('ABC', {'name': '2013-11-07',
+                                          'rate': 20,
+                                          'test': .5}),
+                                 ('ABC', {'name': '2013-11-08',
+                                          'rate': 40,
+                                          'test': .5})]
+                    }
+        self.assertEqual(map_record.values(for_create=True), expected)

=== modified file 'connector/unit/mapper.py'
--- connector/unit/mapper.py	2013-12-24 07:45:03 +0000
+++ connector/unit/mapper.py	2013-12-24 07:45:03 +0000
@@ -19,33 +19,65 @@
 #
 ##############################################################################
 
+"""
+
+Mappers
+=======
+
+Mappers are the ConnectorUnit classes responsible to transform
+external records into OpenERP records and conversely.
+
+"""
+
 import logging
 from collections import namedtuple
+from contextlib import contextmanager
 
 from ..connector import ConnectorUnit, MetaConnectorUnit, Environment
-from ..exception import MappingError
+from ..exception import MappingError, NoConnectorUnitError
 
 _logger = logging.getLogger(__name__)
 
 
 def mapping(func):
-    """ Decorator declarating a mapping for a field """
+    """ Declare that a method is a mapping method.
+
+    It is then used by the :py:class:`Mapper` to convert the records.
+
+    Usage::
+
+        @mapping
+        def any(self, record):
+            return {'output_field': record['input_field']}
+
+    """
     func.is_mapping = True
     return func
 
 
 def changed_by(*args):
-    """ Decorator for the mappings. When fields are modified, we want to modify
-    only the modified fields. Using this decorator, we can specify which fields
-    updates should trigger which mapping.
+    """ Decorator for the mapping methods (:py:func:`mapping`)
+
+    When fields are modified in OpenERP, we want to export only the
+    modified fields. Using this decorator, we can specify which fields
+    updates should trigger which mapping method.
 
     If ``changed_by`` is empty, the mapping is always active.
-    As far as possible, it should be used, thus, when we do an update on
-    only a small number of fields on a record, the size of the output
-    record will be limited to only the fields really having to be
-    modified.
+
+    As far as possible, this decorator should be used for the exports,
+    thus, when we do an update on only a small number of fields on a
+    record, the size of the output record will be limited to only the
+    fields really having to be exported.
+
+    Usage::
+
+        @changed_by('input_field')
+        @mapping
+        def any(self, record):
+            return {'output_field': record['input_field']}
 
     :param *args: field names which trigger the mapping when modified
+
     """
     def register_mapping(func):
         func.changed_by = args
@@ -54,8 +86,19 @@
 
 
 def only_create(func):
-    """ A mapping decorated with ``only_create`` means that it has to be
-    used only for the creation of the records. """
+    """ Decorator for the mapping methods (:py:func:`mapping`)
+
+    A mapping decorated with ``only_create`` means that it has to be
+    used only for the creation of the records.
+
+    Usage::
+
+        @only_create
+        @mapping
+        def any(self, record):
+            return {'output_field': record['input_field']}
+
+    """
     func.only_create = True
     return func
 
@@ -241,6 +284,157 @@
         return cls
 
 
+class MapChild(ConnectorUnit):
+    """ MapChild is responsible to convert items.
+
+    Items are sub-records of a main record.
+    In this example, the items are the records in ``lines``::
+
+        sales = {'name': 'SO10',
+                 'lines': [{'product_id': 1, 'quantity': 2},
+                           {'product_id': 2, 'quantity': 2}]}
+
+    A MapChild is always called from another :py:class:`Mapper` which
+    provides a ``children`` configuration.
+
+    Considering the example above, the "main" :py:class:`Mapper` would
+    returns something as follows::
+
+        {'name': 'SO10',
+                 'lines': [(0, 0, {'product_id': 11, 'quantity': 2}),
+                           (0, 0, {'product_id': 12, 'quantity': 2})]}
+
+    A MapChild is responsible to:
+
+    * Find the :py:class:`Mapper` to convert the items
+    * Eventually filter out some lines (can be done by inheriting
+      :py:meth:`skip_item`)
+    * Convert the items' records using the found :py:class:`Mapper`
+    * Format the output values to the format expected by OpenERP or the
+      backend (as seen above with ``(0, 0, {values})``
+
+    A MapChild can be extended like any other
+    :py:class:`~connector.connector.ConnectorUnit`. However, it is not mandatory
+    to explicitly create a MapChild for each children mapping, the default
+    one will be used (:py:class:`ImportMapChild` or :py:class:`ExportMapChild`).
+
+    The implementation by default does not take care of the updates: if
+    I import a sales order 2 times, the lines will be duplicated. This
+    is not a problem as long as an importation should only support the
+    creation (typical for sales orders). It can be implemented on a
+    case-by-case basis by inheriting :py:meth:`get_item_values` and
+    :py:meth:`format_items`.
+
+    """
+    _model_name = None
+
+    def _child_mapper(self):
+        raise NotImplementedError
+
+    def skip_item(self, map_record):
+        """ Hook to implement in sub-classes when some child
+        records should be skipped.
+
+        The parent record is accessible in ``map_record``.
+        If it returns True, the current child record is skipped.
+
+        :param map_record: record that we are converting
+        :type map_record: :py:class:`MapRecord`
+        """
+        return False
+
+    def get_items(self, items, parent, to_attr, options):
+        """ Returns the formatted output values of items from a main record
+
+        :param items: list of item records
+        :type items: list
+        :param parent: parent record
+        :param to_attr: destination field (can be used for introspecting
+                        the relation)
+        :type to_attr: str
+        :param options: dict of options, herited from the main mapper
+        :return: formatted output values for the item
+
+        """
+        mapper = self._child_mapper()
+        mapped = []
+        for item in items:
+            map_record = mapper.map_record(item, parent=parent)
+            if self.skip_item(map_record):
+                continue
+            mapped.append(self.get_item_values(map_record, to_attr, options))
+        return self.format_items(mapped)
+
+    def get_item_values(self, map_record, to_attr, options):
+        """ Get the raw values from the child Mappers for the items.
+
+        It can be overridden for instance to:
+
+        * Change options
+        * Use a :py:class:`~connector.connector.Binder` to know if an
+          item already exists to modify an existing item, rather than to
+          add it
+
+        :param map_record: record that we are converting
+        :type map_record: :py:class:`MapRecord`
+        :param to_attr: destination field (can be used for introspecting
+                        the relation)
+        :type to_attr: str
+        :param options: dict of options, herited from the main mapper
+
+        """
+        return map_record.values(**options)
+
+    def format_items(self, items_values):
+        """ Format the values of the items mapped from the child Mappers.
+
+        It can be overridden for instance to add the OpenERP
+        relationships commands ``(6, 0, [IDs])``, ...
+
+        As instance, it can be modified to handle update of existing
+        items: check if an 'id' has been defined by
+        :py:meth:`get_item_values` then use the ``(1, ID, {values}``)
+        command
+
+        :param items_values: mapped values for the items
+        :type items_values: list
+
+        """
+        return items_values
+
+
+class ImportMapChild(MapChild):
+    """ :py:class:`MapChild` for the Imports """
+
+    def _child_mapper(self):
+        return self.get_connector_unit_for_model(ImportMapper, self.model._name)
+
+    def format_items(self, items_values):
+        """ Format the values of the items mapped from the child Mappers.
+
+        It can be overridden for instance to add the OpenERP
+        relationships commands ``(6, 0, [IDs])``, ...
+
+        As instance, it can be modified to handle update of existing
+        items: check if an 'id' has been defined by
+        :py:meth:`get_item_values` then use the ``(1, ID, {values}``)
+        command
+
+        :param items_values: list of values for the items to create
+        :type items_values: list
+
+        """
+        return [(0, 0, values) for values in items_values]
+
+
+class ExportMapChild(MapChild):
+    """ :py:class:`MapChild` for the Exports """
+
+
+    def _child_mapper(self):
+        return self.get_connector_unit_for_model(ExportMapper, self.model._name)
+
+
 class Mapper(ConnectorUnit):
     """ A Mapper translates an external record to an OpenERP record and
     conversely. The output of a Mapper is a ``dict``.
@@ -323,7 +517,22 @@
         When a record contains sub-items, like the lines of a sales order,
         we can convert the children using another Mapper::
 
-            children = [('items', 'line_ids', LineMapper)]
+            children = [('items', 'line_ids', 'model.name')]
+
+        It allows to create the sales order and all its lines with the
+        same call to :py:meth:`openerp.osv.orm.BaseModel.create()`.
+
+        When using ``children`` for items of a record, we need to create
+        a :py:class:`Mapper` for the model of the items, and optionally a
+        :py:class:`MapChild`.
+
+    Usage of a Mapper::
+
+        mapper = Mapper(env)
+        map_record = mapper.map_record()
+        values = map_record.values()
+        values = map_record.values(only_create=True)
+        values = map_record.values(fields=['name', 'street'])
 
     """
 
@@ -337,6 +546,8 @@
 
     _map_methods = None
 
+    _map_child_class = None
+
     def __init__(self, environment):
         """
 
@@ -344,15 +555,7 @@
         :type environment: :py:class:`connector.connector.Environment`
         """
         super(Mapper, self).__init__(environment)
-        self._data = None
-        self._data_for_create = None
-        self._data_children = None
-
-    def _init_child_mapper(self, model_name):
-        raise NotImplementedError
-
-    def _after_mapping(self, result):
-        return result
+        self._options = None
 
     def _map_direct(self, record, from_attr, to_attr):
         """ Apply the ``direct`` mappings.
@@ -370,120 +573,185 @@
 
     @property
     def map_methods(self):
+        """ Yield all the methods decorated with ``@mapping`` """
         for meth, definition in self._map_methods.iteritems():
             yield getattr(self, meth), definition
 
-    def _convert(self, record, fields=None, parent_values=None):
-        if fields is None:
-            fields = {}
-
-        self._data = {}
-        self._data_for_create = {}
-        self._data_children = {}
-
-        _logger.debug('converting record %s to model %s', record, self._model_name)
+    def _get_map_child_unit(self, model_name):
+        try:
+            mapper_child = self.get_connector_unit_for_model(
+                self._map_child_class, model_name)
+        except NoConnectorUnitError:
+            # does not force developers to use a MapChild ->
+            # will use the default one if not explicitely defined
+            env = Environment(self.backend_record,
+                              self.session,
+                              model_name)
+            mapper_child = self._map_child_class(env)
+        return mapper_child
+
+    def _map_child(self, map_record, from_attr, to_attr, model_name):
+        """ Convert items of the record as defined by children """
+        assert self._map_child_class is not None, "_map_child_class required"
+        child_records = map_record.source[from_attr]
+        mapper_child = self._get_map_child_unit(model_name)
+        items = mapper_child.get_items(child_records, map_record,
+                                       to_attr, options=self.options)
+        return items
+
+    @contextmanager
+    def _mapping_options(self, options):
+        """ Change the mapping options for the Mapper.
+
+        Context Manager to use in order to alter the behavior
+        of the mapping, when using ``_apply`` or ``finalize``.
+
+        """
+        current = self._options
+        self._options = options
+        yield
+        self._options = current
+
+    @property
+    def options(self):
+        """ Options can be accessed in the mapping methods with
+        ``self.options``. """
+        return self._options
+
+    def map_record(self, record, parent=None):
+        """ Get a :py:class:`MapRecord` with record, ready to be
+        converted using the current Mapper.
+
+        :param record: record to transform
+        :param parent: optional parent record, for items
+
+        """
+        return MapRecord(self, record, parent=parent)
+
+    def _apply(self, map_record, options=None):
+        """ Apply the mappings on a :py:class:`MapRecord`
+
+        :param map_record: source record to convert
+        :type map_record: :py:class:`MapRecord`
+
+        """
+        if options is None:
+            options = {}
+        with self._mapping_options(options):
+            return self._apply_with_options(map_record)
+
+    def _apply_with_options(self, map_record):
+        """ Apply the mappings on a :py:class:`MapRecord` with
+        contextual options (the ``options`` given in
+        :py:meth:`MapRecord.values()` are accessible in
+        ``self.options``)
+
+        :param map_record: source record to convert
+        :type map_record: :py:class:`MapRecord`
+
+        """
+        assert self.options is not None, (
+            "options should be defined with '_mapping_options'")
+        _logger.debug('converting record %s to model %s',
+                      map_record.source, self.model._name)
+
+        fields = self.options.fields
+        for_create = self.options.for_create
+        result = {}
         for from_attr, to_attr in self.direct:
             if (not fields or from_attr in fields):
-                value = self._map_direct(record,
+                value = self._map_direct(map_record.source,
                                          from_attr,
                                          to_attr)
-                self._data[to_attr] = value
+                result[to_attr] = value
 
         for meth, definition in self.map_methods:
             changed_by = definition.changed_by
             if (not fields or not changed_by or
                     changed_by.intersection(fields)):
-                values = meth(record)
+                values = meth(map_record.source)
                 if not values:
                     continue
                 if not isinstance(values, dict):
                     raise ValueError('%s: invalid return value for the '
                                      'mapping method %s' % (values, meth))
-                if definition.only_create:
-                    self._data_for_create.update(values)
-                else:
-                    self._data.update(values)
+                if not definition.only_create or for_create:
+                    result.update(values)
 
         for from_attr, to_attr, model_name in self.children:
             if (not fields or from_attr in fields):
-                self._map_child(record, from_attr, to_attr, model_name)
-
-    def skip_convert_child(self, record, parent_values=None):
-        """ Hook to implement in sub-classes when some child
-        records should be skipped.
-
-        This method is only relevable for child mappers.
-
-        If it returns True, the current child record is skipped."""
-        return False
+                result[to_attr] = self._map_child(map_record, from_attr,
+                                                  to_attr, model_name)
+
+        return self.finalize(map_record, result)
+
+    def finalize(self, map_record, values):
+        """ Called at the end of the mapping.
+
+        Can be used to modify the values before returning them, as the
+        ``on_change``.
+
+        :param map_record: source map_record
+        :type map_record: :py:class:`MapRecord`
+        :param values: mapped values
+        :returns: mapped values
+        :rtype: dict
+        """
+        return values
+
+    def _after_mapping(self, result):
+        """ .. deprecated:: 2.1 """
+        raise DeprecationWarning('Mapper._after_mapping() has been deprecated, '
+                                 'use Mapper.finalize()')
 
     def convert_child(self, record, parent_values=None):
-        """ Transform child row contained in a main record, only
-        called from another Mapper.
-
-        :param parent_values: openerp record of the containing object
-            (e.g. sale_order for a sale_order_line)
-        """
-        self._convert(record, parent_values=parent_values)
+        """ .. deprecated:: 2.1 """
+        raise DeprecationWarning('Mapper.convert_child() has been deprecated, '
+                                 'use Mapper.map_record() then '
+                                 'map_record.values() ')
 
     def convert(self, record, fields=None):
-        """ Transform an external record to an OpenERP record or the opposite
-
-        Sometimes we want to map values only when we create the records.
-        The mapping methods have to be decorated with ``only_create`` to
-        map values only for creation of records.
-
-        :param record: record to transform
-        :param fields: list of fields to convert, if empty, all fields
-                       are converted
+        """ .. deprecated:: 2.1
+               See :py:meth:`Mapper.map_record`, :py:meth:`MapRecord.values`
         """
-        self._convert(record, fields=fields)
+        raise DeprecationWarning('Mapper.convert() has been deprecated, '
+                                 'use Mapper.map_record() then '
+                                 'map_record.values() ')
 
     @property
     def data(self):
-        """ Returns a dict for a record processed by
-        :py:meth:`~_convert` """
-        if self._data is None:
-            raise ValueError('Mapper.convert should be called before '
-                             'accessing the data')
-        result = self._data.copy()
-        for attr, mappers in self._data_children.iteritems():
-            child_data = [mapper.data for mapper in mappers]
-            if child_data:
-                result[attr] = self._format_child_rows(child_data)
-        return self._after_mapping(result)
+        """ .. deprecated:: 2.1
+               See :py:meth:`Mapper.map_record`, :py:meth:`MapRecord.values`
+        """
+        raise DeprecationWarning('Mapper.data has been deprecated, '
+                                 'use Mapper.map_record() then '
+                                 'map_record.values() ')
 
     @property
     def data_for_create(self):
-        """ Returns a dict for a record processed by
-        :py:meth:`~_convert` to use only for creation of the record. """
-        if self._data is None:
-            raise ValueError('Mapper.convert should be called before '
-                             'accessing the data')
-        result = self._data.copy()
-        result.update(self._data_for_create)
-        for attr, mappers in self._data_children.iteritems():
-            child_data = [mapper.data_for_create for mapper in mappers]
-            if child_data:
-                result[attr] = self._format_child_rows(child_data)
-        return self._after_mapping(result)
-
-    def _format_child_rows(self, child_records):
-        return child_records
-
-    def _map_child(self, record, from_attr, to_attr, model_name):
-        child_records = record[from_attr]
-        self._data_children[to_attr] = []
-        for child_record in child_records:
-            mapper = self._init_child_mapper(model_name)
-            if mapper.skip_convert_child(child_record, parent_values=record):
-                continue
-            mapper.convert_child(child_record, parent_values=record)
-            self._data_children[to_attr].append(mapper)
+        """ .. deprecated:: 2.1
+               See :py:meth:`Mapper.map_record`, :py:meth:`MapRecord.values`
+        """
+        raise DeprecationWarning('Mapper.data_for_create has been deprecated, '
+                                 'use Mapper.map_record() then '
+                                 'map_record.values() ')
+
+    def skip_convert_child(self, record, parent_values=None):
+        """ .. deprecated:: 2.1
+               Use :py:meth:`MapChild.skip_item` instead.
+        """
+        raise DeprecationWarning('Mapper.skip_convert_child has been deprecated, '
+                                 'use MapChild.skip_item().')
 
 
 class ImportMapper(Mapper):
-    """ Transform a record from a backend to an OpenERP record """
+    """ :py:class:`Mapper` for imports.
+
+    Transform a record from a backend to an OpenERP record
+
+    """
+
+    _map_child_class = ImportMapChild
 
     def _map_direct(self, record, from_attr, to_attr):
         """ Apply the ``direct`` mappings.
@@ -501,25 +769,25 @@
         if not value:
             return False
 
-        # may be replaced by the explicit backend_to_m2o
+        # Backward compatibility: when a field is a relation, and a modifier is
+        # not used, we assume that the relation model is a binding.
+        # Use an explicit modifier backend_to_m2o in the 'direct' mappings to
+        # change that.
         column = self.model._all_columns[to_attr].column
         if column._type == 'many2one':
-            mapping = backend_to_m2o(from_attr, binding=True)
+            mapping = backend_to_m2o(from_attr)
             value = mapping(self, record, to_attr)
         return value
 
-    def _init_child_mapper(self, model_name):
-        env = Environment(self.backend_record,
-                          self.session,
-                          model_name)
-        return env.get_connector_unit(ImportMapper)
-
-    def _format_child_rows(self, child_records):
-        return [(0, 0, data) for data in child_records]
-
 
 class ExportMapper(Mapper):
-    """ Transform a record from OpenERP to a backend record """
+    """ :py:class:`Mapper` for exports.
+
+    Transform a record from OpenERP to a backend record
+
+    """
+
+    _map_child_class = ExportMapChild
 
     def _map_direct(self, record, from_attr, to_attr):
         """ Apply the ``direct`` mappings.
@@ -537,15 +805,130 @@
         if not value:
             return False
 
-        # may be replaced by the explicit m2o_to_backend
+        # Backward compatibility: when a field is a relation, and a modifier is
+        # not used, we assume that the relation model is a binding.
+        # Use an explicit modifier m2o_to_backend  in the 'direct' mappings to
+        # change that.
         column = self.model._all_columns[from_attr].column
         if column._type == 'many2one':
-            mapping = m2o_to_backend(from_attr, binding=True)
+            mapping = m2o_to_backend(from_attr)
             value = mapping(self, record, to_attr)
         return value
 
-    def _init_child_mapper(self, model_name):
-        env = Environment(self.backend_record,
-                          self.session,
-                          model_name)
-        return env.get_connector_unit(ExportMapper)
+
+class MapRecord(object):
+    """ A record prepared to be converted using a :py:class:`Mapper`.
+
+    MapRecord instances are prepared by :py:meth:`Mapper.map_record`.
+
+    Usage::
+
+        mapper = SomeMapper(env)
+        map_record = mapper.map_record()
+        output_values = map_record.values()
+
+    See :py:meth:`values` for more information on the available arguments.
+
+    """
+
+    def __init__(self, mapper, source, parent=None):
+        self._source = source
+        self._mapper = mapper
+        self._parent = parent
+        self._forced_values = {}
+
+    @property
+    def source(self):
+        """ Source record to be converted """
+        return self._source
+
+    @property
+    def parent(self):
+        """ Parent record if the current record is an item """
+        return self._parent
+
+    def values(self, for_create=None, fields=None, **kwargs):
+        """ Build and returns the mapped values according to the options.
+
+        Usage::
+
+            mapper = SomeMapper(env)
+            map_record = mapper.map_record()
+            output_values = map_record.values()
+
+        Creation of records
+            When using the option ``for_create``, only the mappings decorated
+            with ``@only_create`` will be mapped.
+
+            ::
+
+                output_values = map_record.values(for_create=True)
+
+        Filter on fields
+            When using the ``fields`` argument, the mappings will be
+            filtered using either the source key in ``direct`` arguments,
+            either the ``changed_by`` arguments for the mapping methods.
+
+            ::
+
+                output_values = map_record.values(fields=['name', 'street'])
+
+        Custom options
+            Arbitrary key and values can be defined in the ``kwargs``
+            arguments.  They can later be used in the mapping methods
+            using ``self.options``.
+
+            ::
+
+                output_values = map_record.values(tax_include=True)
+
+        :param for_create: specify if only the mappings for creation
+                           (``@only_create``) should be mapped.
+        :type for_create: boolean
+        :param fields: filter on fields
+        :type fields: list
+        :param **kwargs: custom options, they can later be used in the
+                         mapping methods
+
+        """
+        options = MapOptions(for_create=for_create, fields=fields, **kwargs)
+        values = self._mapper._apply(self, options=options)
+        values.update(self._forced_values)
+        return values
+
+    def update(self, *args, **kwargs):
+        """ Force values to be applied after a mapping.
+
+        Usage::
+
+            mapper = SomeMapper(env)
+            map_record = mapper.map_record()
+            map_record.update(a=1)
+            output_values = map_record.values()
+            # output_values will at least contain {'a': 1}
+
+        The values assigned with ``update()`` are in any case applied,
+        they have a greater priority than the mapping values.
+
+        """
+        self._forced_values.update(*args, **kwargs)
+
+
+class MapOptions(dict):
+    """ Container for the options of mappings.
+
+    Offer the convenience to access options with attributes
+
+    """
+
+    def __getitem__(self, key):
+        try:
+            return super(MapOptions, self).__getitem__(key)
+        except KeyError:
+            return None
+
+    def __getattr__(self, key):
+        return self[key]
+
+    def __setattr__(self, key, value):
+        self[key] = value