Guewen Baconnier @ Camptocamp has proposed merging 
lp:~c2c/openobject-server/trunk-sparse-field-akretion-integrity into 
lp:~openerp-dev/openobject-server/trunk-sparse-field-akretion.

Requested reviews:
  Olivier Dony (OpenERP) (odo-openerp)

For more details, see:
https://code.launchpad.net/~c2c/openobject-server/trunk-sparse-field-akretion-integrity/+merge/86044

Hello Olivier,

I tried to address an integrity problem in this merge proposal.

I'm quite not sure I've taken the good approach and not sure that my code is 
good enough to be merged.
So I would like to have your expertise your help to address this issue in the 
best way possible.

Integrity problem :

The sparse fields may be many2one and many2many, but there is no "set null", 
"cascade" or constraint to not delete the linked resources. So when you delete 
a resource linked by a many2one sparse field, the relation will stay in the 
serialized field.
Ie. 
 - I have a "option" sparse field on product.template, stored in 
my_serialized_field. That's a many2one on a model product.option
 - On a product A, I write the option to use the Option A (id: 10 in 
product.option), so in my_serialized_field, I'll have : {'option': 10}
 - I delete the Option A in product.option. my_serialized_field is still 
{'option': 10}.
 - When I'll try to read product.template with field option, I will have an 
error because the linked resource does no longer exist.

Precisely in fields.py postprocess method:
         if field_type == "many2one":
             # make the result a tuple if it is not already one
             if isinstance(value, (int,long)) and hasattr(obj._columns[field], 
'relation'):
                 obj_model = obj.pool.get(obj._columns[field].relation)
                 dict_names = dict(obj_model.name_get(cr, uid, [value], 
context))
                 result = (value, dict_names[value])

name_get is called on a non-existing relation.


In the best case, we would like to handle the integrity like postgresql does, 
so with set null, cascade or block when you delete the linked resource (the 
option in my upper use case).
But I fear that will just kill the performance.

So my approach was to keep the id in the serialized field (we have to keep in 
mind that its content may lies), and when we read its value, if the resource is 
not found, we clean its value.


What do you think about that ?

Is it still expected to be merged in 6.1 ?

Thanks !
Guewen
-- 
https://code.launchpad.net/~c2c/openobject-server/trunk-sparse-field-akretion-integrity/+merge/86044
Your team OpenERP R&D Team is subscribed to branch 
lp:~openerp-dev/openobject-server/trunk-sparse-field-akretion.
=== modified file 'openerp/osv/fields.py'
--- openerp/osv/fields.py	2011-11-17 14:30:10 +0000
+++ openerp/osv/fields.py	2011-12-16 14:02:26 +0000
@@ -1003,6 +1003,11 @@
             return []
         return self._fnct_search(obj, cr, uid, obj, name, args, context=context)
 
+    def _make_tuple(self, cr, uid, obj, field, value, context=None):
+        obj_model = obj.pool.get(obj._columns[field].relation)
+        dict_names = dict(obj_model.name_get(cr, uid, [value], context))
+        return value, dict_names[value]
+
     def postprocess(self, cr, uid, obj, field, value=None, context=None):
         if context is None:
             context = {}
@@ -1011,9 +1016,7 @@
         if field_type == "many2one":
             # make the result a tuple if it is not already one
             if isinstance(value, (int,long)) and hasattr(obj._columns[field], 'relation'):
-                obj_model = obj.pool.get(obj._columns[field].relation)
-                dict_names = dict(obj_model.name_get(cr, uid, [value], context))
-                result = (value, dict_names[value])
+                result = self._make_tuple(cr, uid, obj, field, value, context=context)
 
         if field_type == 'binary':
             if context.get('bin_size', False):
@@ -1031,13 +1034,18 @@
             result = float(value)
         return result
 
+    def _get_multi(self, cr, uid, obj, id, data, context=None):
+        res = {}
+        for field, value in data.iteritems():
+            if value:
+                res[field] = self.postprocess(cr, uid, obj, field, value, context)
+        return res
+
     def get(self, cr, obj, ids, name, uid=False, context=None, values=None):
         result = self._fnct(obj, cr, uid, ids, name, self._arg, context)
         for id in ids:
             if self._multi and id in result:
-                for field, value in result[id].iteritems():
-                    if value:
-                        result[id][field] = self.postprocess(cr, uid, obj, field, value, context)
+                result[id].update(self._get_multi(cr, uid, obj, id, result[id], context=context))
             elif result.get(id):
                 result[id] = self.postprocess(cr, uid, obj, name, result[id], context)
         return result
@@ -1229,6 +1237,21 @@
             return read_value
         return value
 
+    def _make_tuple(self, cr, uid, obj, field, value, context=None):
+        obj_model = obj.pool.get(obj._columns[field].relation)
+        dict_names = dict(obj_model.name_get(cr, uid, [value], context))
+        if not dict_names:
+            return
+        return value, dict_names[value]
+
+    def _get_multi(self, cr, uid, obj, id, data, context=None):
+        result = super(sparse, self)._get_multi(cr, uid, obj, id, data, context=context)
+        # Integrity check
+        for field, value in data.iteritems():
+            if value and result.get(field) is None:
+                result.pop(field, None)
+                obj.write(cr, uid, id, {field: False}, context=context)
+        return result
 
     def _fnct_write(self,obj,cr, uid, ids, field_name, value, args, context=None):
         if not type(ids) == list:

_______________________________________________
Mailing list: https://launchpad.net/~openerp-dev-gtk
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~openerp-dev-gtk
More help   : https://help.launchpad.net/ListHelp

Reply via email to