Merge lp:~sebastien.beau/openerp-connector/openerp-connector-fix-useless-fire-sparse-fields into lp:~openerp-connector-core-editors/openerp-connector/7.0

[Sébastien BEAU - http://www.akretion.com]

Hi
When we write in a sparse fields (it's inherit the function fields) OpenERP will write into the serialized fields.
This mean that if I update X sparse fields on the product, the producer will fire X+1 even and so X+1 job.

The easy way to fix it is to not fire an even if the key updated is only the serialized fields.

What do you think?

Thanks

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Hi,

Thanks for your MP.

Can you add the test as well please (for a write in a serialized field too)?

Could I propose to rename 'keys' to 'fields'?

I think we can have a more clever fix for not much work. I'm maybe wrong without investigating further, but I'm sure you'll stop me in such case ;-)

If we write directly in a serialized field, we know the fields we are writing on.
I would propose to:
Fire vals.keys() + serialized.keys(), unless if '__do_not_fire_serialized' is present in the context. '__do_not_fire_serialized' has to be put in the context given to the `write_original` method.

Thus,
* if we write directly to sparse fields -> ok because the many writes on the serialized will be skipped due to the presence of the sentinel '__do_not_fire_serialized'
* If we write directly to a serialized field -> ok because the fields keys will be present in the first fire

review: Needs Fixing (code review, no test)

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

> <...>
> Fire vals.keys() + serialized.keys(), unless if '__do_not_fire_serialized' is present in the context. >'__do_not_fire_serialized' has to be put in the context given to the

Should be actually something as follows :
    fields = vals.keys()
    for each serialized field: # pseudo code
        fields = set(vals.keys()).update(serialized.keys())
        fields.remove(field_serialized)
where serialized is the the dict of serialized vals and field_serialized the name of the field

> <...>
>
> Thus,
> * if we write directly to sparse fields -> ok because the many writes on the
> serialized will be skipped due to the presence of the sentinel
> '__do_not_fire_serialized'
> * If we write directly to a serialized field -> ok because the fields keys
> will be present in the first fire

Hmm I inversed some things here, but I think you understood me.

Note that your fix is failing if we update 2 fields where 1 of them is a serialized.

When I

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

This project is now hosted on https://github.com/OCA/connector. Please move your proposal there if you still want to merge it once fixed. This guide may help you https://github.com/OCA/maintainers-tools/wiki/How-to-move-a-Merge-Proposal-to-GitHub

Unmerged revisions

579. By Sébastien BEAU - http://www.akretion.com on 2013-07-16

[FIX] fix useless fire on when writing on a sparse fields

=== modified file 'connector/producer.py'
--- connector/producer.py	2013-04-23 13:27:22 +0000
+++ connector/producer.py	2013-07-16 15:57:28 +0000
@@ -55,9 +55,12 @@
             ids = [ids]
         session = ConnectorSession(cr, uid, context=context)
         if on_record_write.has_consumer_for(session, self._name):
+            keys = vals.keys()
+            if len(keys) == 1 and self._columns[keys[0]]._type == 'serialized':
+                return result
             for record_id in ids:
                 on_record_write.fire(session, self._name,
-                                     record_id, vals.keys())
+                                     record_id, keys)
     return result
 orm.Model.write = write