From 7db7db8301055b411abe9ffdb4201cac5a22313d Mon Sep 17 00:00:00 2001 From: Mourad Date: Fri, 23 Mar 2018 14:56:52 +0100 Subject: [PATCH 1/5] [WIP] sale_exception_by_domain --- base_exception/models/base_exception.py | 37 ++++++++++++++++++-- base_exception/views/base_exception_view.xml | 8 ++++- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/base_exception/models/base_exception.py b/base_exception/models/base_exception.py index e845f9c8c..35e7198ae 100644 --- a/base_exception/models/base_exception.py +++ b/base_exception/models/base_exception.py @@ -41,6 +41,11 @@ class ExceptionRule(models.Model): model = fields.Selection( selection=[], string='Apply on', required=True) + type = fields.Selection( + selection=[('by_domain', 'By domain'), + ('by_py_code', 'By Python Code')], + string='Exception Type', required=True) + domain = fields.Char('Domain') active = fields.Boolean('Active') code = fields.Text( 'Python Code', @@ -65,6 +70,19 @@ 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) + + @api.onchange('type',) + def onchange_type(self): + if self.type == 'by_domain': + self.code = False + elif self.type == 'by_py_code': + self.domain = False + class BaseException(models.AbstractModel): _name = 'base.exception' @@ -141,6 +159,7 @@ class BaseException(models.AbstractModel): def detect_exceptions(self): """returns the list of exception_ids for all the considered base.exceptions """ + import pdb; pdb.set_trace() if not self: return [] exception_obj = self.env['exception.rule'] @@ -195,10 +214,17 @@ class BaseException(models.AbstractModel): def _detect_exceptions(self, model_exceptions, sub_exceptions): self.ensure_one() + import pdb; pdb.set_trace() exception_ids = [] for rule in model_exceptions: - if self._rule_eval(rule, self.rule_group, self): + if rule.type == 'by_py_code' and self._rule_eval( + rule, self.rule_group, self): exception_ids.append(rule.id) + elif rule.type == 'by_domain' and rule.domain: + domain = rule._get_domain() + domain.append(('id', '=', self.id)) + if self.search(domain): + exception_ids.append(rule.id) if sub_exceptions: for obj_line in self._get_lines(): for rule in sub_exceptions: @@ -208,8 +234,15 @@ class BaseException(models.AbstractModel): # (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): + if rule.type == 'by_py_code' and self._rule_eval( + rule, group_line, obj_line): exception_ids.append(rule.id) + elif rule.type == 'by_domain' and rule.domain: + domain = rule._get_domain() + domain.append(('id', '=', obj_line.id)) + if obj_line.search(domain): + exception_ids.append(rule.id) + return exception_ids @implemented_by_base_exception diff --git a/base_exception/views/base_exception_view.xml b/base_exception/views/base_exception_view.xml index a44e376e5..cb63c3af5 100644 --- a/base_exception/views/base_exception_view.xml +++ b/base_exception/views/base_exception_view.xml @@ -31,7 +31,13 @@ - + + + From 34ae095f2c898acace0a6cdcc95ce8e4dca8fa69 Mon Sep 17 00:00:00 2001 From: hparfr Date: Tue, 30 Oct 2018 10:54:47 +0100 Subject: [PATCH 2/5] add execution rule based instead of record based improve the perfs dramastically when there is a lot of records --- base_exception/README.rst | 1 + base_exception/__manifest__.py | 2 +- base_exception/models/base_exception.py | 193 ++++++++++++++----- base_exception/views/base_exception_view.xml | 6 +- 4 files changed, 150 insertions(+), 52 deletions(-) diff --git a/base_exception/README.rst b/base_exception/README.rst index ff7be0580..19015bec9 100644 --- a/base_exception/README.rst +++ b/base_exception/README.rst @@ -48,6 +48,7 @@ Contributors * Yannick Vaucher * SodexisTeam * Mourad EL HADJ MIMOUNE +* Raphaël Reverdy Maintainer ---------- diff --git a/base_exception/__manifest__.py b/base_exception/__manifest__.py index 422e24dfb..ad5b430c6 100644 --- a/base_exception/__manifest__.py +++ b/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, ...)""", diff --git a/base_exception/models/base_exception.py b/base_exception/models/base_exception.py index 35e7198ae..6f344cfb2 100644 --- a/base_exception/models/base_exception.py +++ b/base_exception/models/base_exception.py @@ -41,16 +41,20 @@ class ExceptionRule(models.Model): model = fields.Selection( selection=[], string='Apply on', required=True) - type = fields.Selection( + exception_type = fields.Selection( selection=[('by_domain', 'By domain'), - ('by_py_code', 'By Python Code')], - string='Exception Type', required=True) + ('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. @@ -76,11 +80,11 @@ class ExceptionRule(models.Model): self.ensure_one() return safe_eval(self.domain) - @api.onchange('type',) - def onchange_type(self): - if self.type == 'by_domain': + @api.onchange('exception_type',) + def onchange_exception_type(self): + if self.exception_type == 'by_domain': self.code = False - elif self.type == 'by_py_code': + elif self.exception_type == 'by_py_code': self.domain = False @@ -155,30 +159,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 """ - import pdb; pdb.set_trace() 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): @@ -211,39 +260,87 @@ class BaseException(models.AbstractModel): return space.get('failed', False) @api.multi - def _detect_exceptions(self, model_exceptions, - sub_exceptions): - self.ensure_one() - import pdb; pdb.set_trace() - 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 rule.type == 'by_py_code' and self._rule_eval( - rule, self.rule_group, self): - exception_ids.append(rule.id) - elif rule.type == 'by_domain' and rule.domain: - domain = rule._get_domain() - domain.append(('id', '=', self.id)) - if self.search(domain): - exception_ids.append(rule.id) - if sub_exceptions: - for obj_line in self._get_lines(): - for rule in sub_exceptions: - if rule.id in exception_ids: - # 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 rule.type == 'by_py_code' and self._rule_eval( - rule, group_line, obj_line): - exception_ids.append(rule.id) - elif rule.type == 'by_domain' and rule.domain: - domain = rule._get_domain() - domain.append(('id', '=', obj_line.id)) - if obj_line.search(domain): - exception_ids.append(rule.id) - - return exception_ids + 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: + group_line = rec.rule_group + '_line' + for obj_line in rec._get_lines(): + for rule in sub_exceptions: + 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 + 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): diff --git a/base_exception/views/base_exception_view.xml b/base_exception/views/base_exception_view.xml index cb63c3af5..12aefb4c9 100644 --- a/base_exception/views/base_exception_view.xml +++ b/base_exception/views/base_exception_view.xml @@ -31,12 +31,12 @@ - + From 252ee4362080241a24682057cf5f6d80d0d25279 Mon Sep 17 00:00:00 2001 From: Hpar Date: Thu, 8 Nov 2018 16:13:38 +0100 Subject: [PATCH 3/5] Remove onchange on exception_type No more empty fields after changing type. --- base_exception/models/base_exception.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/base_exception/models/base_exception.py b/base_exception/models/base_exception.py index 6f344cfb2..10afb15e1 100644 --- a/base_exception/models/base_exception.py +++ b/base_exception/models/base_exception.py @@ -80,13 +80,6 @@ class ExceptionRule(models.Model): self.ensure_one() return safe_eval(self.domain) - @api.onchange('exception_type',) - def onchange_exception_type(self): - if self.exception_type == 'by_domain': - self.code = False - elif self.exception_type == 'by_py_code': - self.domain = False - class BaseException(models.AbstractModel): _name = 'base.exception' From 7d2f7d6644a4f760783759a88587a25c313a32c8 Mon Sep 17 00:00:00 2001 From: Mourad EL HADJ MIMOUNE Date: Tue, 27 Nov 2018 15:09:23 +0100 Subject: [PATCH 4/5] [FIX] rename migration floder according to the new version --- .../migrations/10.0.2.0.0/pre-migration.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 base_exception/migrations/10.0.2.0.0/pre-migration.py diff --git a/base_exception/migrations/10.0.2.0.0/pre-migration.py b/base_exception/migrations/10.0.2.0.0/pre-migration.py new file mode 100644 index 000000000..35600697f --- /dev/null +++ b/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')]) From c640d3e6af9fbed791358d80a23fc08b5230ad7f Mon Sep 17 00:00:00 2001 From: Hpar Date: Mon, 10 Dec 2018 10:52:01 +0100 Subject: [PATCH 5/5] Update readme Add a note about safe_eval --- base_exception/README.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/base_exception/README.rst b/base_exception/README.rst index 19015bec9..5b53f15f8 100644 --- a/base_exception/README.rst +++ b/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 ------