Data corruption: fields.related writes to the target model with source id

Bug #915975 reported by Stefan Rijnhart (Opener)
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Odoo Server (MOVED TO GITHUB)
Confirmed
Medium
OpenERP's Framework R&D
Therp Backports (Deprecated)
Fix Released
High
Stefan Rijnhart (Opener)
Server-6.1
New
Undecided
Unassigned
Server-7.0
New
Undecided
Unassigned

Bug Description

Hi,

Problem:
------------
A fields.related with type='many2many' tries to write to the target model but with id of the source model in case the original value of the many2many field on the target resource is empty.

Analysis:
-----------
Take for example a column definition in osv class 'source.model':

  'related_field_id': fields.related('target_model_id', 'target_field', type='many2many'),

A value is written to this field. The current value on the target model is empty.

The method related._fnct_write()[1] from osv/fields.py loops over ('target_model_id', 'target_field'). The second iteration is supposed to retrieve the resource id of the target model. It does so by setting 't_id' to t_data['id'] in three different places (!) in the loop, where 't_data' is the browse object of the target resource. It still fails to do so in the circumstances described above.

Although the code is a bit difficult to read, you can check that *if* the current value of the field (t_data[self.arg[i]])[2] is empty *and* type is in ('one2many', 'many2one')[3], t_id does not get set to t_data['id'] but keeps its original value instead. This original value is the resource id of the source model set in the first iteration of the loop.

Result:
---------
If a resource exists with this id, the new value get silently written to this wrong resource. If such a resource does not exist, the following error occurs: "'One of the records you are trying to modify has already been deleted".

This problem occurs on openerp-server/6.0 (rev. 3565). Trunk has not been tested, but the function in which the problem occurs has remained unchanged.

[1] http://bazaar.launchpad.net/~openerp/openobject-server/6.0/view/head:/bin/osv/fields.py?start_revid=3566#L863
[2] http://bazaar.launchpad.net/~openerp/openobject-server/6.0/view/head:/bin/osv/fields.py?start_revid=3566#L874
[3] http://bazaar.launchpad.net/~openerp/openobject-server/6.0/view/head:/bin/osv/fields.py?start_revid=3566#L875

Revision history for this message
Jignesh Rathod(OpenERP) (jir-openerp) wrote :
Revision history for this message
Jignesh Rathod(OpenERP) (jir-openerp) wrote :

Hello Stefan Rijnhart ,

I have checked this issue at my end.But I did not face any problem
as you mention in bug report.So would you please provide more information
regarding this issue.

I have attached a video for your more reference.

Correct me If I am wrong..

Thanks and waiting for reply!

Changed in openobject-server:
status: New → Incomplete
Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Hi Jignesh,

thank you for your attention on this issue!

The fields that you demonstrate are regular many2many fields. The problem does not occur in that case. The problem only occurs with fields.related of type 'many2many'. I have created a module that demonstrates the problem, and I will attach a video with spoken word to talk you through the problem.

You can get the module here to see for yourself:

http://bazaar.launchpad.net/~stefan-therp/junk/lp915975-fields-related-many2many/files

Cheers,
Stefan.

Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Additional info: the demonstration video was made on the OpenERP 6.1 release. During the demonstration it seemed that the problem only occurs if the many2many field contained no records yet.

Changed in openobject-server:
status: Incomplete → New
Revision history for this message
Jignesh Rathod(OpenERP) (jir-openerp) wrote :
Revision history for this message
Jignesh Rathod(OpenERP) (jir-openerp) wrote :

Hello Stefan ,

Yes,You are right , I have seen some inconsistancy in this situation.

Let me explain:
---------------------------

We have created M2M related field with type "M2M".

When I have added a record in releted field then its removed which is
the first inconstancy(See which shows in video).

I have added record in regular "M2M" field then it is affact (attached) on related field
which is correct.

Now I have removed this record from related field, then same record is also
removed from the regular field,So again I said it also inconsistant behaviour.

Lets compare this behaviour M2O field,

I have checked this same behaviour in 6.0(M2O relation on Hr_employee object)
and removed the readonly="1" from parent_id field.Now you can see manager field is related
with department when I am changing the department ,Manager will change and
If I remove the manager from there, Department will as it is.One more thing is I change manager
field is does't affect to the department field.

From this behaviour M2O is working fine and for M2M related field have inconsitant
behaviour,That's why I am confirming this issue also I have attached video which is describe
the problem more clearly.

Thanks for understanding!

Changed in openobject-server:
assignee: nobody → OpenERP's Framework R&D (openerp-dev-framework)
importance: Undecided → Low
status: New → Confirmed
Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Encountered the bug again, now on one2many target fields.

Changed in therp-backports:
importance: Undecided → High
assignee: nobody → Stefan Rijnhart (Therp) (stefan-therp)
milestone: none → 6.1-maintenance
status: New → Confirmed
summary: - Fields.related writes to the target model with source id
+ Data corruption: fields.related writes to the target model with source
+ id
Revision history for this message
Serpent Consulting Services (serpent-consulting-services) wrote :

+1 Stefan,

We have faced a similar issue on Mass editing, while we edited country for a partner !

Related M2O of O2M is a problem too!

We have fixed it in same method, but I doubt our solution is yet tested for all cases.

Thanks.

Changed in openobject-server:
importance: Low → Medium
Changed in therp-backports:
status: Confirmed → Fix Released
Lara (Therp) (lfreeke)
Changed in therp-backports:
milestone: 6.1-maintenance → 7.0-maintenance
Lara (Therp) (lfreeke)
Changed in therp-backports:
milestone: 7.0-maintenance → none
Revision history for this message
Serpent Consulting Services (serpent-consulting-services) wrote :

I doubt when would this bug get noticed!

Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

The affected code was refactored in OpenERP 7.0 similarly to the proposed solution, so it looks like the bug does not affect this release anymore (but I haven't tested it).

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.