Merge pull request #1410 from akretion/10-sale_exception_by_domain

[10.0][IMP] base_exception add rules by domain and improve perfs

base_exception/README.rst

@@ -32,6 +32,7 @@ Roadmap
 -------
 Terms used in old api like `pool`, `cr`, `uid` must be removed porting this module in version 12.
 
+This module execute user provided code though a safe_eval, it's unsecure? How mitigate risks should be adressed in future versions of this module.
 
 Images
 ------
@@ -48,6 +49,7 @@ Contributors
 * Yannick Vaucher <yannick.vaucher@camptocamp.com>
 * SodexisTeam <dev@sodexis.com>
 * Mourad EL HADJ MIMOUNE <mourad.elhadj.mimoune@akretion.com>
+* Raphaël Reverdy <raphael.reverdy@akretion.com>
 
 Maintainer
 ----------

base_exception/__manifest__.py

@@ -3,7 +3,7 @@
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
 
 {'name': 'Exception Rule',
- 'version': '10.0.1.0.0',
+ 'version': '10.0.2.0.0',
  'category': 'Generic Modules',
  'summary': """This module provide an abstract model to manage customizable
   exceptions to be applied on different models (sale order, invoice, ...)""",

base_exception/migrations/10.0.2.0.0/pre-migration.py

@@ -0,0 +1,12 @@
+# -*- coding: utf-8 -*-
+# © 2017 Akretion, Mourad EL HADJ MIMOUNE
+# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html).
+
+from openupgradelib import openupgrade
+
+
+@openupgrade.migrate(use_env=True)
+def migrate(env, version):
+    cr = env.cr
+    openupgrade.rename_tables(cr, [('sale_exception', 'exception_rule')])
+    openupgrade.rename_models(cr, [('sale.exception', 'exception.rule')])

base_exception/models/base_exception.py

@@ -41,11 +41,20 @@ class ExceptionRule(models.Model):
     model = fields.Selection(
         selection=[],
         string='Apply on', required=True)
+    exception_type = fields.Selection(
+        selection=[('by_domain', 'By domain'),
+                   ('by_py_code', 'By python code')],
+        string='Exception Type', required=True, default='by_py_code',
+        help="By python code: allow to define any arbitrary check\n"
+             "By domain: limited to a selection by an odoo domain:\n"
+             "           performance can be better when exceptions "
+             "           are evaluated with several records")
+    domain = fields.Char('Domain')
     active = fields.Boolean('Active')
     code = fields.Text(
         'Python Code',
         help="Python code executed to check if the exception apply or "
-             "not. The code must apply block = True to apply the "
+             "not. The code must apply failed = True to apply the "
              "exception.",
         default="""
 # Python code. Use failed = True to block the base.exception.
@@ -65,6 +74,12 @@ class ExceptionRule(models.Model):
 #  - context: current context
 """)
 
+    @api.multi
+    def _get_domain(self):
+        """ override me to customize domains according exceptions cases """
+        self.ensure_one()
+        return safe_eval(self.domain)
+
 
 class BaseException(models.AbstractModel):
     _name = 'base.exception'
@@ -137,29 +152,75 @@ class BaseException(models.AbstractModel):
             return False
         return True
 
+    @api.multi
+    def _reverse_field(self):
+        """Name of the many2many field from exception rule to self.
+
+        In order to take advantage of domain optimisation, exception rule
+        model should have a many2many field to inherited object.
+        The opposit relation already exists in the name of exception_ids
+
+        Example:
+        class ExceptionRule(models.Model):
+            _inherit = 'exception.rule'
+
+            model = fields.Selection(
+            selection_add=[
+                ('sale.order', 'Sale order'),
+                [...]
+            ])
+            sale_ids = fields.Many2many(
+                'sale.order',
+                string='Sales')
+            [...]
+        """
+        exception_obj = self.env['exception.rule']
+        reverse_fields = self.env['ir.model.fields'].search([
+            ['model', '=', 'exception.rule'],
+            ['ttype', '=', 'many2many'],
+            ['relation', '=', self[0]._name],
+        ])
+        # ir.model.fields may contain old variable name
+        # so we check if the field exists on exception rule
+        return ([
+            field.name for field in reverse_fields
+            if hasattr(exception_obj, field.name)
+        ] or [None])[0]
+
     @api.multi
     def detect_exceptions(self):
-        """returns the list of exception_ids for all the considered base.exceptions
+        """List all exception_ids applied on self
+        Exception ids are also written on records
         """
         if not self:
             return []
         exception_obj = self.env['exception.rule']
         all_exceptions = exception_obj.sudo().search(
             [('rule_group', '=', self[0].rule_group)])
+        # TODO fix self[0] : it may not be the same on all ids in self
         model_exceptions = all_exceptions.filtered(
             lambda ex: ex.model == self._name)
         sub_exceptions = all_exceptions.filtered(
             lambda ex: ex.model != self._name)
 
+        reverse_field = self._reverse_field()
+        if reverse_field:
+            optimize = True
+        else:
+            optimize = False
+
+        exception_by_rec, exception_by_rule = self._detect_exceptions(
+            model_exceptions, sub_exceptions, optimize)
+
         all_exception_ids = []
-        for obj in self:
-            if obj.ignore_exception:
-                continue
-            exception_ids = obj._detect_exceptions(
-                model_exceptions, sub_exceptions)
+        for obj, exception_ids in exception_by_rec.iteritems():
             obj.exception_ids = [(6, 0, exception_ids)]
             all_exception_ids += exception_ids
-        return all_exception_ids
+        for rule, exception_ids in exception_by_rule.iteritems():
+            rule[reverse_field] = [(6, 0, exception_ids.ids)]
+            if exception_ids:
+                all_exception_ids += [rule.id]
+        return list(set(all_exception_ids))
 
     @api.model
     def _exception_rule_eval_context(self, obj_name, rec):
@@ -192,25 +253,87 @@ class BaseException(models.AbstractModel):
         return space.get('failed', False)
 
     @api.multi
-    def _detect_exceptions(self, model_exceptions,
-                           sub_exceptions):
-        self.ensure_one()
-        exception_ids = []
+    def _detect_exceptions(
+            self, model_exceptions, sub_exceptions,
+            optimize=False,
+    ):
+        """Find exceptions found on self.
+
+        @returns
+            exception_by_rec: (record_id, exception_ids)
+            exception_by_rule: (rule_id, record_ids)
+        """
+        exception_by_rec = {}
+        exception_by_rule = {}
+        exception_set = set()
+        python_rules = []
+        dom_rules = []
+        optim_rules = []
+
         for rule in model_exceptions:
-            if self._rule_eval(rule, self.rule_group, self):
-                exception_ids.append(rule.id)
+            if rule.exception_type == 'by_py_code':
+                python_rules.append(rule)
+            elif rule.exception_type == 'by_domain' and rule.domain:
+                if optimize:
+                    optim_rules.append(rule)
+                else:
+                    dom_rules.append(rule)
+
+        for rule in optim_rules:
+            domain = rule._get_domain()
+            domain.append(['ignore_exception', '=', False])
+            domain.append(['id', 'in', self.ids])
+            records_with_exception = self.search(domain)
+            exception_by_rule[rule] = records_with_exception
+            if records_with_exception:
+                exception_set.add(rule.id)
+
+        if len(python_rules) or len(dom_rules) or sub_exceptions:
+            for rec in self:
+                for rule in python_rules:
+                    if (
+                        not rec.ignore_exception and
+                        self._rule_eval(rule, rec.rule_group, rec)
+                    ):
+                        exception_by_rec.setdefault(rec, []).append(rule.id)
+                        exception_set.add(rule.id)
+                for rule in dom_rules:
+                    # there is no reverse many2many, so this rule
+                    # can't be optimized, see _reverse_field
+                    domain = rule._get_domain()
+                    domain.append(['ignore_exception', '=', False])
+                    domain.append(['id', '=', rec.id])
+                    if self.search_count(domain):
+                        exception_by_rec.setdefault(
+                            rec, []).append(rule.id)
+                        exception_set.add(rule.id)
                 if sub_exceptions:
-            for obj_line in self._get_lines():
+                    group_line = rec.rule_group + '_line'
+                    for obj_line in rec._get_lines():
                         for rule in sub_exceptions:
-                    if rule.id in exception_ids:
-                        # we do not matter if the exception as already been
+                            if rule.id in exception_set:
+                                # we do not matter if the exception as
+                                # already been
                                 # found for an line of this object
                                 # (ex sale order line if obj is sale order)
                                 continue
-                    group_line = self.rule_group + '_line'
-                    if self._rule_eval(rule, group_line, obj_line):
-                        exception_ids.append(rule.id)
-        return exception_ids
+                            if rule.exception_type == 'by_py_code':
+                                if self._rule_eval(
+                                    rule, group_line, obj_line
+                                ):
+                                    exception_by_rec.setdefault(
+                                        rec, []).append(rule.id)
+                            elif (
+                                rule.exception_type == 'by_domain' and
+                                rule.domain
+                            ):
+                                # sub_exception are currently not optimizable
+                                domain = rule._get_domain()
+                                domain.append(('id', '=', obj_line.id))
+                                if obj_line.search_count(domain):
+                                    exception_by_rec.setdefault(
+                                        rec, []).append(rule.id)
+        return exception_by_rec, exception_by_rule
 
     @implemented_by_base_exception
     def _get_lines(self):

base_exception/views/base_exception_view.xml

@@ -31,7 +31,13 @@
                     <group colspan="4" col="2" groups="base.group_system">
                         <field name="rule_group"/>
                         <field name="model"/>
-                        <field name="code"/>
+                        <field name="exception_type" widget="radio"/>
+                        <field name="domain" 
+                            attrs="{'invisible': [('exception_type','!=','by_domain')]}
+                            "/>
+                        <field name="code"
+                            attrs="{'invisible': [('exception_type','!=','by_py_code')]}
+                            "/>
                     </group>
                 </form>
             </field>