[6.0] osv.memory objects may be created with wrong field values for dynamic fields in some cases

[Raphaël Valyi - http://www.akretion.com]

Hello,

this is on trunk server rev #2635 using addons rev #4734

osv.memory objects are not properly written from the client into the memory and are also wrongly read if there are several instancse of the same osv.memory class. This is very annoying as it lead to totally wrong behavior when doing twice the same action in the same server session for instance.

A simple way to reproduce the bug is with the sale and stock module + demo data.

1) create a new sale order, add one product line with say product "ATX" and 12 items, validate the order.
2) go to the related expedition picking and force the expedition.
3) click on the right side "return picking" of the expedition picking to return products.
Just specify you want to return 1 product only.
Check that you have a return with only 1 product, the first time it works fine.

Do the exact same process again without restarting the server. Yes, your return picking will now have 12 products, not just 1 and you'll also get a strange popup after you do the picking.

What is happening?
To discover I instrumented a bit stock/wizard/stock/wizard/stock_return_picking.py and enabled the GTK client XML/RPC logs.
Here is my instrumentation patch:
=== modified file 'stock/wizard/stock_return_picking.py'
--- stock/wizard/stock_return_picking.py 2010-08-10 12:28:52 +0000
+++ stock/wizard/stock_return_picking.py 2010-08-19 20:54:38 +0000
@@ -148,6 +148,11 @@
         date_cur = time.strftime('%Y-%m-%d %H:%M:%S')

         move_ids = [m.id for m in [line for line in pick.move_lines]]
+ print "self", self
+ print "ids", ids
+ print "picking", pick.id, pick.name
+ print "move_ids", move_ids
+ print "self.read(cr, uid, ids[0])", data
         set_invoice_state_to_none = True
         for move in move_obj.browse(cr, uid, move_ids):
             if not new_picking:

Now let's run the described scenario again while looking the logs:

===== 1st iteration:
client-side XML/RPC:
DEBUG_RPC:rpc.request:('execute', 'consig', 1, 'admin', ('stock.return.picking', 'create', {'return133': 1.0, 'invoice_state': 'none'}, {'lang': u'en_US', 'search_default_available': 1, 'tz': False, 'active_model': u'stock.picking', 'contact_display': 'partner', 'active_ids': [94], 'active_id': 94}))
...

server side:
self <stock.wizard.stock_return_picking.stock_return_picking object at 0x30dda10>
ids [1]
picking 94 OUT/00032
move_ids [133]
self.read(cr, uid, ids[0]) {'return133': 1.0, 'id': 1, 'invoice_state': 'none'}

===== 2nd iteration:
client-side XML/RPC:
DEBUG_RPC:rpc.request:('execute', 'consig', 1, 'admin', ('stock.return.picking', 'create', {'return135': 1.0, 'invoice_state': 'none'}, {'lang': u'en_US', 'search_default_available': 1, 'tz': False, 'active_model': u'stock.picking', 'contact_display': 'partner', 'active_ids': [96], 'active_id': 96}))
...

server side:
self <stock.wizard.stock_return_picking.stock_return_picking object at 0x30dda10>
ids [2]
picking 96 OUT/00033
move_ids [135]
self.read(cr, uid, ids[0]) {'return133': False, 'invoice_state': 'none', 'id': 2, 'return135': 12.0}

So as you see, my second picking id 96 has only one move id 135. In the GTK clients things are all right.
But, when doing a self.read(cr, uid, [2]), the server read get totally wrong values: it remembers the 'return133' param while it's False now, and 'return135' is left to 12.0 while it is supposed to be assigned at 1.0, just like in the previous case.

So it looks like create/write/read behave wrongly when there are several instance of an osv_memory. This leads to critical bugs as you can't do a return here for instance!

Hope this helps, again this is critical.

[Borja López Soilán (NeoPolus) (borjals)]

I can confirm this bug for 5.2dev (yeah, it is that old).

When we where testing/developing the account_chart_update module and the pxgo_account_admin_tools (extra-addons). We made them compatible with 5.2dev back then and we noticed some problems like this one: https://bugs.launchpad.net/openobject-client/+bug/586252

But aside from that bug, we also noticed that, for wizards that created osv_memory records linked to the main wizard, if the wizards were run again, they would show up duplicated/unexpected records: To prevent this from happening we have to redefine the search method for osv_memory objects and filter by hand the osv_memory objects with the correct 'wizard_id'.

I don't know if the problem is in the way the ORM is mapping/filtering the osv_memory records, or that it doesn't clean some parameters (remember the context={} problem?) or something like that, but in the end osv_memory wizards didn't work properly on 5.2dev and still look half-broken on 6.0dev.

[Olivier Dony (Odoo) (odo-openerp)]

@Raphael
Thanks a lot for the detailed bug report, that was quite helpful!
After analysis, the behavior you are seeing is not a global problem with all osv_memory objects, but a combination of several factors:
- The osv_memory object has dynamic fields, generated at runtime by fields_view_get or fields_get or similarly
- The osv_memory object overrides default_get() to provide default values for its dynamic fields, but does so incorrectly and returns values for fields that have not been requested in the parameters of default_get

In that case, the ORM may wrongly use the default value of the dynamic fields instead of those passed to create().

You may want to try the minimalist (and incomplete) patch attached, to partially fix the wizard and see if that solves your problem.

I'm working on a better patch of the wizard and a patch of the ORM to guard against faulty default_get() implementations like this one.

@Borja
I will check your other bug report 586252, but you also mention issues with duplicate/unexpected results in search() (which bug 586252 is not about)? Could you please indicate some specific bug or steps to reproduce?
Thanks!

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

Olivier,

it looks like your made your patch with the RML engine or something like that ;-) (it's full of strange characters)
Could you please regenerate it?

[Olivier Dony (Odoo) (odo-openerp)]

Oops, I piped the output of colored diff instead of just diff, so you have ANSI color codes in it ;-)
Here's the colorless one...

[Olivier Dony (Odoo) (odo-openerp)]

OK, the fixes have landed in :
- server-trunk at rev. 2645 <email address hidden>
- addons-trunk at rev. 4778 <email address hidden>

Both these revisions will fix your case independently, but the server update fixes other potential cases.