Merge lp:~openerp-dev/openobject-server/6.0-server-fix-stored-function-triggers into lp:openobject-server/6.0

[Olivier Dony (Odoo)]

Rewrite of ORM's _store_get_values() used to generate the list of stored function fields that need to be recomputed as a result of any create/write/unlink operation.

This rewrite attempts to make the code a bit more readable (had a headache just reading the previous version), but most importantly changes the logic to respect *all* priorities set on the function fields's triggers.
The previous version of this code only considered the priority of the first function field found for each model, hence causing hard-to-diagnose issues during updates of objects with complex inter-dependent fields.

This changes allows fixing bug 715470 as explained in the related merge proposal for addons:
https://code.launchpad.net/~openerp-dev/openobject-addons/trunk-bug-715470-ara/+merge/51288

Once merged in 6.0, this fix will be forward-ported to trunk.

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

Note that the comments on top about the structure of _inherits and _inherit_fields are not really related to this issue, they're just provided for reference.

Unmerged revisions

3367. By Olivier Dony (Odoo) on 2011-03-15

[FIX] orm: rewrote and fixed _store_get_values to properly respect priority

_store_get_values is in charge of gathering the list of
function fields that should be recomputed as a result
of any write() operation, due to store flag triggers.
The previous version would group fields by model and
only respect the priority of the first trigger for a
given model, creating various problems when priority
of field computation is important.
The new version tries to be more readable and respect
all priorities.

=== modified file 'bin/osv/orm.py'
--- bin/osv/orm.py	2011-03-11 08:27:20 +0000
+++ bin/osv/orm.py	2011-03-17 10:02:26 +0000
@@ -396,7 +396,15 @@
     _order = 'id'
     _sequence = None
     _description = None
+
+    # structure:
+    #  { 'parent_model': 'm2o_field', ... }
     _inherits = {}
+
+    # structure:
+    #  { 'field_name': ('parent_model', 'm2o_field_to_reach_parent' , field_column_obj), ... }
+    _inherit_fields = None
+
     _table = None
     _invalids = set()
     _log_create = False
@@ -2803,7 +2811,7 @@
                         if f == order:
                             ok = False
                 if ok:
-                    self.pool._store_function[object].append( (self._name, store_field, fnct, fields2, order, length))
+                    self.pool._store_function[object].append((self._name, store_field, fnct, tuple(fields2) if fields2 else None, order, length))
                     self.pool._store_function[object].sort(lambda x, y: cmp(x[4], y[4]))
 
         for (key, _, msg) in self._sql_constraints:
@@ -3692,44 +3700,52 @@
 
            :return: [(priority, model_name, [record_ids,], [function_fields,])]
         """
-        # FIXME: rewrite, cleanup, use real variable names
-        # e.g.: http://pastie.org/1222060
-        result = {}
-        fncts = self.pool._store_function.get(self._name, [])
-        for fnct in range(len(fncts)):
-            if fncts[fnct][3]:
-                ok = False
-                if not fields:
-                    ok = True
-                for f in (fields or []):
-                    if f in fncts[fnct][3]:
-                        ok = True
-                        break
-                if not ok:
-                    continue
-
-            result.setdefault(fncts[fnct][0], {})
-
+        stored_functions = self.pool._store_function.get(self._name, [])
+
+        # use indexed names for the details of the stored_functions:
+        model_name_, func_field_to_compute_, id_mapping_fnct_, trigger_fields_, priority_ = range(5)
+
+        # only keep functions that should be triggered for the ``fields``
+        # being written to.
+        if fields is None:
+            # fields == None when unlink() is being called
+            to_compute = stored_functions
+        else:
+            to_compute = [f for f in stored_functions \
+                if ((not f[trigger_fields_]) or set(fields).intersection(f[trigger_fields_]))]
+
+        mapping = {}
+        for function in to_compute:
             # uid == 1 for accessing objects having rules defined on store fields
-            ids2 = fncts[fnct][2](self, cr, 1, ids, context)
-            for id in filter(None, ids2):
-                result[fncts[fnct][0]].setdefault(id, [])
-                result[fncts[fnct][0]][id].append(fnct)
-        dict = {}
-        for object in result:
-            k2 = {}
-            for id, fnct in result[object].items():
-                k2.setdefault(tuple(fnct), [])
-                k2[tuple(fnct)].append(id)
-            for fnct, id in k2.items():
-                dict.setdefault(fncts[fnct[0]][4], [])
-                dict[fncts[fnct[0]][4]].append((fncts[fnct[0]][4], object, id, map(lambda x: fncts[x][1], fnct)))
-        result2 = []
-        tmp = dict.keys()
-        tmp.sort()
-        for k in tmp:
-            result2 += dict[k]
-        return result2
+            target_ids = [id for id in function[id_mapping_fnct_](self, cr, 1, ids, context) if id]
+
+            # the compound key must consider the priority and model name
+            key = (function[priority_], function[model_name_])
+            for target_id in target_ids:
+                mapping.setdefault(key, {}).setdefault(target_id,set()).add(tuple(function))
+
+        # Here mapping looks like:
+        # { (10, 'model_a') : { target_id1: [ (function_1_tuple, function_2_tuple) ], ... }
+        #   (20, 'model_a') : { target_id2: [ (function_3_tuple, function_4_tuple) ], ... }
+        #   (99, 'model_a') : { target_id1: [ (function_5_tuple, function_6_tuple) ], ... }
+        # }
+
+        # Now we need to generate the batch function calls list
+        # call_map =
+        #   { (10, 'model_a') : [(10, 'model_a', [record_ids,], [function_fields,])] }
+        call_map = {}
+        for ((priority,model), id_map) in mapping.iteritems():
+            functions_ids_maps = {}
+            # function_ids_maps =
+            #   { (function_1_tuple, function_2_tuple) : [target_id1, target_id2, ..] }
+            for id, functions in id_map.iteritems():
+                functions_ids_maps.setdefault(tuple(functions), []).append(id)
+            for functions, ids in functions_ids_maps.iteritems():
+                call_map.setdefault((priority,model),[]).append((priority, model, ids, [f[func_field_to_compute_] for f in functions]))
+        ordered_keys = call_map.keys()
+        ordered_keys.sort()
+        result = reduce(operator.add, (call_map[k] for k in ordered_keys)) if call_map else []
+        return result
 
     def _store_set_values(self, cr, uid, ids, fields, context):
         """Calls the fields.function's "implementation function" for all ``fields``, on records with ``ids`` (taking care of