[IMP] base/res_users: improve reading/writing of reified group fields

- simplify the code by removing unused cases
- do not modify input argument 'value' of create/write
- do not call BaseModel.read() with reified group fields (this causes warnings)

openerp/addons/base/res/res_users.py

@@ -22,6 +22,7 @@
 import itertools
 import logging
 from functools import partial
+from itertools import repeat
 
 from lxml import etree
 from lxml.builder import E
@@ -650,25 +651,30 @@ class users_implied(osv.osv):
 # Naming conventions for reified groups fields:
 # - boolean field 'in_group_ID' is True iff
 #       ID is in 'groups_id'
-# - boolean field 'in_groups_ID1_..._IDk' is True iff
-#       any of ID1, ..., IDk is in 'groups_id'
 # - selection field 'sel_groups_ID1_..._IDk' is ID iff
 #       ID is in 'groups_id' and ID is maximal in the set {ID1, ..., IDk}
 #----------------------------------------------------------
 
-def name_boolean_group(id): return 'in_group_' + str(id)
+def name_boolean_group(id):
-def name_boolean_groups(ids): return 'in_groups_' + '_'.join(map(str, ids))
+    return 'in_group_' + str(id)
-def name_selection_groups(ids): return 'sel_groups_' + '_'.join(map(str, ids))
+
+def name_selection_groups(ids):
+    return 'sel_groups_' + '_'.join(map(str, ids))
+
+def is_boolean_group(name):
+    return name.startswith('in_group_')
+
+def is_selection_groups(name):
+    return name.startswith('sel_groups_')
 
-def is_boolean_group(name): return name.startswith('in_group_')
-def is_boolean_groups(name): return name.startswith('in_groups_')
-def is_selection_groups(name): return name.startswith('sel_groups_')
 def is_reified_group(name):
-    return is_boolean_group(name) or is_boolean_groups(name) or is_selection_groups(name)
+    return is_boolean_group(name) or is_selection_groups(name)
 
-def get_boolean_group(name): return int(name[9:])
+def get_boolean_group(name):
-def get_boolean_groups(name): return map(int, name[10:].split('_'))
+    return int(name[9:])
-def get_selection_groups(name): return map(int, name[11:].split('_'))
+
+def get_selection_groups(name):
+    return map(int, name[11:].split('_'))
 
 def partition(f, xs):
     "return a pair equivalent to (filter(f, xs), filter(lambda x: not f(x), xs))"
@@ -789,45 +795,39 @@ class users_view(osv.osv):
     _inherit = 'res.users'
 
     def create(self, cr, uid, values, context=None):
-        self._set_reified_groups(values)
+        values = self._remove_reified_groups(values)
         return super(users_view, self).create(cr, uid, values, context)
 
     def write(self, cr, uid, ids, values, context=None):
-        self._set_reified_groups(values)
+        values = self._remove_reified_groups(values)
         return super(users_view, self).write(cr, uid, ids, values, context)
 
-    def _set_reified_groups(self, values):
+    def _remove_reified_groups(self, values):
-        """ reflect reified group fields in values['groups_id'] """
+        """ return `values` without reified group fields """
-        if 'groups_id' in values:
+        add, rem = [], []
-            # groups are already given, ignore group fields
+        values1 = {}
-            for f in filter(is_reified_group, values.iterkeys()):
-                del values[f]
-            return
 
-        add, remove = [], []
+        for key, val in values.iteritems():
-        for f in values.keys():
+            if is_boolean_group(key):
-            if is_boolean_group(f):
+                (add if val else rem).append(get_boolean_group(key))
-                target = add if values.pop(f) else remove
+            elif is_selection_groups(key):
-                target.append(get_boolean_group(f))
+                rem += get_selection_groups(key)
-            elif is_boolean_groups(f):
+                if val:
-                if not values.pop(f):
+                    add.append(val)
-                    remove.extend(get_boolean_groups(f))
+            else:
-            elif is_selection_groups(f):
+                values1[key] = val
-                remove.extend(get_selection_groups(f))
+
-                selected = values.pop(f)
+        if 'groups_id' not in values and (add or rem):
-                if selected:
+            # remove group ids in `rem` and add group ids in `add`
-                    add.append(selected)
+            values1['groups_id'] = zip(repeat(3), rem) + zip(repeat(4), add)
-        # update values *only* if groups are being modified, otherwise
+
-        # we introduce spurious changes that might break the super.write() call.
+        return values1
-        if add or remove:
-            # remove groups in 'remove' and add groups in 'add'
-            values['groups_id'] = [(3, id) for id in remove] + [(4, id) for id in add]
 
     def default_get(self, cr, uid, fields, context=None):
         group_fields, fields = partition(is_reified_group, fields)
         fields1 = (fields + ['groups_id']) if group_fields else fields
         values = super(users_view, self).default_get(cr, uid, fields1, context)
-        self._get_reified_groups(group_fields, values)
+        self._add_reified_groups(group_fields, values)
 
         # add "default_groups_ref" inside the context to set default value for group_id with xml values
         if 'groups_id' in fields and isinstance(context.get("default_groups_ref"), list):
@@ -846,29 +846,35 @@ class users_view(osv.osv):
         return values
 
     def read(self, cr, uid, ids, fields=None, context=None, load='_classic_read'):
-        fields_get = fields if fields is not None else self.fields_get(cr, uid, context=context).keys()
+        # determine whether reified groups fields are required, and which ones
-        group_fields, _ = partition(is_reified_group, fields_get)
+        fields1 = fields or self.fields_get(cr, uid, context=context).keys()
+        group_fields, other_fields = partition(is_reified_group, fields1)
 
-        inject_groups_id = group_fields and fields and 'groups_id' not in fields
+        # read regular fields (other_fields); add 'groups_id' if necessary
-        if inject_groups_id:
+        drop_groups_id = False
-            fields.append('groups_id')
+        if group_fields and fields:
-        res = super(users_view, self).read(cr, uid, ids, fields, context=context, load=load)
+            if 'groups_id' not in other_fields:
+                other_fields.append('groups_id')
+                drop_groups_id = True
+        else:
+            other_fields = fields
 
-        if res and group_fields:
+        res = super(users_view, self).read(cr, uid, ids, other_fields, context=context, load=load)
+
+        # post-process result to add reified group fields
+        if group_fields:
             for values in (res if isinstance(res, list) else [res]):
-                self._get_reified_groups(group_fields, values)
+                self._add_reified_groups(group_fields, values)
-                if inject_groups_id:
+                if drop_groups_id:
                     values.pop('groups_id', None)
         return res
 
-    def _get_reified_groups(self, fields, values):
+    def _add_reified_groups(self, fields, values):
-        """ compute the given reified group fields from values['groups_id'] """
+        """ add the given reified group fields into `values` """
         gids = set(parse_m2m(values.get('groups_id') or []))
         for f in fields:
             if is_boolean_group(f):
                 values[f] = get_boolean_group(f) in gids
-            elif is_boolean_groups(f):
-                values[f] = not gids.isdisjoint(get_boolean_groups(f))
             elif is_selection_groups(f):
                 selected = [gid for gid in get_selection_groups(f) if gid in gids]
                 values[f] = selected and selected[-1] or False