Merge lp:~camptocamp/openobject-addons/extra-trunk-base_external_referentials-subexport-no-cr into lp:openobject-addons/extra-trunk

[Guewen Baconnier @ Camptocamp]

Hi,

This fix the issue:

https://bugs.launchpad.net/magentoerpconnect/+bug/1068174

You'll find the explanation of why the bug occurs in the report.

Alexandre, you'll see what I meant by sub-transactions, even if there are not sub-transactions at database level, obviously, transactions are strictly parallel on the database point of view, but we really nest the exports. I had a misuse of language, but the the result is the same, we are not able to see transaction B from transaction A, because B is opened, committed & closed between the opening, committing & closing of transaction A.

My proposal is based on the current logic used in the import.

When a `export_no_new_cr` key is present in the context, it keeps the current transaction, and that sounds logical because if the export of the category fails, I expect that the export of the product (which wanted to export the category) fails too.

But, this implementation is sensible because if the context is not properly passed all the chain of calls, a new transaction would be opened and we could have the same sort of issues.

I think we would better have to apply the reverse logic, so to never open a new transaction, unless it is asked, and modify the top level exports and imports to use this new method or argument.
But it would require much more changes on the actual code base.

I have one doubt concerning the case when we are in a sub-export and we get an error. We do not rollback the cr, it has to be rollbacked by the main export. We have to raise the exception again to propagate it.
Normally, the `log_failed()` method should do it, because when we are in a sub-export, this latter raise when the key `use_external_log` is not in the context, and it is removed from the context for sub-exports. That's a bit tricky but it should do it.
Again, the problem would be if the context is not properly passed.

Unmerged revisions

5823. By Guewen Baconnier @ Camptocamp <email address hidden> on 2012-10-18

[FIX] issue lp:1068174 records created on magento during an sub-export are not visible in the export

When a new transaction is opened for an export and this export have sub-exports (exports in the mappings for example),
the sub-exports must not open new transactions, otherwise, the transaction of the main export will not be able
to read the data written by the sub-exports (external ids created principally).

This implementation is sensible because the `export_no_new_cr` has to be propagated through the context,
and if the context chain is broken, it will open a new transaction regardless. It would maybe be better in
the other way, no new cr by default, and explicitely ask for a new cr on top level exports, but it would require
to search and change all the top level export, and do the same for the imports to have a consistant logic between them.

=== modified file 'base_external_referentials/external_osv.py'
--- base_external_referentials/external_osv.py	2012-07-25 14:06:17 +0000
+++ base_external_referentials/external_osv.py	2012-10-18 15:44:20 +0000
@@ -536,6 +536,9 @@
             raise osv.except_osv(_('External Import Error'), _("The object imported need an external_id, maybe the mapping doesn't exist for the object : %s" %self._name))
         external_id = vals['external_id']
         del vals['external_id']
+        _logger.debug(
+            "Starting import of external resource %s with external id %s %s new cursor %s",
+            self._name, external_id, 'without' if record_cr is None else 'with', record_cr or cr)
         cid, wid = self._ext_import_one(
             record_cr or cr, uid, external_id,
             vals,
@@ -819,18 +822,26 @@
 
     report_line_obj = self.pool.get('external.report.line')
 
-    export_ctx = dict(context)
+    export_ctx = dict(context, export_no_new_cr=True)
     # avoid to use external logs in submethods as they are handle at this level
     export_ctx.pop('use_external_log', False)
 
-    record_cr = pooler.get_db(cr.dbname).cursor()
+    record_cr = None
+    if not context.get('export_no_new_cr'):
+        record_cr = pooler.get_db(cr.dbname).cursor()
+
     cid = wid = False
+    _logger.debug(
+        "Starting export of resource %s with openerp id %s %s new cursor %s",
+        self._name, record_data['id'], 'without' if record_cr is None else 'with', record_cr or cr)
     try:
         cid, wid = self._ext_export_one(
-            record_cr, uid, record_data, referential_id, defaults=defaults, context=export_ctx)
+            record_cr or cr, uid, record_data, referential_id, defaults=defaults, context=export_ctx)
     except (MappingError, osv.except_osv, xmlrpclib.Fault):
         _logger.exception('error during export of %s (id: %s) with referential_id %s. Creating a report line', record_data.get('name', 'NONAME'), record_data.get('id'), referential_id)
-        record_cr.rollback()
+        if record_cr:
+            record_cr.rollback()
+
         report_line_obj.log_failed(
             cr, uid,
             self._name,
@@ -840,7 +851,8 @@
             defaults=defaults,
             context=context)
     else:
-        record_cr.commit()
+        if record_cr:
+            record_cr.commit()
         report_line_obj.log_success(
             cr, uid,
             self._name,
@@ -849,7 +861,8 @@
             res_id=record_data['id'],
             context=context)
     finally:
-        record_cr.close()
+        if record_cr:
+            record_cr.close()
     return cid, wid
 
 def ext_export(self, cr, uid, ids, external_referential_ids=[], defaults={}, context=None):