From 5ca66d5ccf03312e59eefc5d1eb9a5b5613e83dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Krier?= Date: Sat, 12 May 2018 00:45:55 +0200 Subject: [PATCH] Add tree mixin - Implement generic get_rec_name and search_rec_name - Include recursive check issue7383 review48281002 --- CHANGELOG | 1 + doc/ref/models/models.rst | 27 ++++- trytond/ir/ui/menu.py | 48 +-------- trytond/model/__init__.py | 3 +- trytond/model/modelstorage.py | 43 -------- trytond/model/tree.py | 116 +++++++++++++++++++++ trytond/tests/__init__.py | 2 + trytond/tests/mptt.py | 9 +- trytond/tests/test_tree.py | 191 ++++++++++++++++++++++++++++++++++ trytond/tests/tree.py | 35 +++++++ 10 files changed, 374 insertions(+), 101 deletions(-) create mode 100644 trytond/model/tree.py create mode 100644 trytond/tests/test_tree.py create mode 100644 trytond/tests/tree.py diff --git a/CHANGELOG b/CHANGELOG index 090f6107bb..39b8519a59 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,4 @@ +* Add tree mixin * Rename "install-dependences" into "activate-dependencies" * Use recursive common table expression for child_of/parent_of operators * Add sql_cast on Field diff --git a/doc/ref/models/models.rst b/doc/ref/models/models.rst index cb6dead356..c806862d63 100644 --- a/doc/ref/models/models.rst +++ b/doc/ref/models/models.rst @@ -334,11 +334,6 @@ Class methods: modification of data coming from XML files. This method must be overiden to change this behavior. -.. classmethod:: ModelStorage.check_recursion(records[, parent]) - - Helper method that checks if there is no recursion in the tree composed - with ``parent`` as parent field name. - .. classmethod:: ModelStorage.validate(records) Validate the integrity of records after creation and modification. This @@ -818,3 +813,25 @@ Class attributes are: .. _mixin: http://en.wikipedia.org/wiki/Mixin .. _JSON: http://en.wikipedia.org/wiki/Json .. _UNION: http://en.wikipedia.org/wiki/Union_(SQL)#UNION_operator + +==== +tree +==== + +.. method:: tree([parent[, name[, separator]]]) + +Returns a mixin_ class :class:`TreeMixin`. `parent` indicates the name of the +field that defines the parent of the tree and its default value is `parent`. +`name` indicates the name of the field that defines the name of the record and +its default value is `name`. If `separator` is set, the +:meth:`ModelStorage.get_rec_name` constructs the name by concatenating each +parent names using it as separator and :meth:`ModelStorage.search_rec_name` is +adapted to search across the tree. + + +.. class:: TreeMixin + +.. classmethod:: TreeMixin.check_recursion(records) + + Helper method that checks if there is no recursion in the tree defined by + :meth:`tree`. diff --git a/trytond/ir/ui/menu.py b/trytond/ir/ui/menu.py index 5e52803dad..c7cfe6d48e 100644 --- a/trytond/ir/ui/menu.py +++ b/trytond/ir/ui/menu.py @@ -3,7 +3,7 @@ from itertools import groupby from trytond.model import ( - ModelView, ModelSQL, DeactivableMixin, fields, sequence_ordered) + ModelView, ModelSQL, DeactivableMixin, fields, sequence_ordered, tree) from trytond.transaction import Transaction from trytond.tools import grouped_slice from trytond.pool import Pool @@ -68,10 +68,10 @@ CLIENT_ICONS = [(x, x) for x in ( 'tryton-system', 'tryton-undo', 'tryton-web-browser')] -SEPARATOR = ' / ' -class UIMenu(DeactivableMixin, sequence_ordered(), ModelSQL, ModelView): +class UIMenu(DeactivableMixin, sequence_ordered(), tree(separator=' / '), + ModelSQL, ModelView): "UI menu" __name__ = 'ir.ui.menu' @@ -96,14 +96,6 @@ class UIMenu(DeactivableMixin, sequence_ordered(), ModelSQL, ModelView): 'Action Keywords') favorite = fields.Function(fields.Boolean('Favorite'), 'get_favorite') - @classmethod - def __setup__(cls): - super(UIMenu, cls).__setup__() - cls._error_messages.update({ - 'wrong_name': ('"%%s" is not a valid menu name because it is ' - 'not allowed to contain "%s".' % SEPARATOR), - }) - @classmethod def order_complete_name(cls, tables): return cls.name.convert_order('name', tables, cls) @@ -123,40 +115,6 @@ class UIMenu(DeactivableMixin, sequence_ordered(), ModelSQL, ModelView): return sorted(CLIENT_ICONS + [(name, name) for _, name in Icon.list_icons()]) - @classmethod - def validate(cls, menus): - super(UIMenu, cls).validate(menus) - cls.check_recursion(menus) - for menu in menus: - menu.check_name() - - def check_name(self): - if SEPARATOR in self.name: - self.raise_user_error('wrong_name', (self.name,)) - - def get_rec_name(self, name): - parent = self.parent - name = self.name - while parent: - name = parent.name + SEPARATOR + name - parent = parent.parent - return name - - @classmethod - def search_rec_name(cls, name, clause): - if isinstance(clause[2], basestring): - values = clause[2].split(SEPARATOR.strip()) - values.reverse() - domain = [] - field = 'name' - for name in values: - domain.append((field, clause[1], name.strip())) - field = 'parent.' + field - else: - domain = [('name',) + tuple(clause[1:])] - ids = [m.id for m in cls.search(domain, order=[])] - return [('parent', 'child_of', ids)] - @classmethod def search_global(cls, text): # TODO improve search clause diff --git a/trytond/model/__init__.py b/trytond/model/__init__.py index 58f665ebd6..1dc2d12016 100644 --- a/trytond/model/__init__.py +++ b/trytond/model/__init__.py @@ -13,9 +13,10 @@ from .multivalue import MultiValueMixin, ValueMixin from .descriptors import dualmethod from .order import sequence_ordered from .active import DeactivableMixin +from .tree import tree __all__ = ['Model', 'ModelView', 'ModelStorage', 'ModelSingleton', 'ModelSQL', 'Check', 'Unique', 'Exclude', 'Workflow', 'DictSchemaMixin', 'MatchMixin', 'UnionMixin', 'dualmethod', 'MultiValueMixin', 'ValueMixin', - 'EvalEnvironment', 'sequence_ordered', 'DeactivableMixin'] + 'EvalEnvironment', 'sequence_ordered', 'DeactivableMixin', 'tree'] diff --git a/trytond/model/modelstorage.py b/trytond/model/modelstorage.py index 9f06da2be4..69d94a492a 100644 --- a/trytond/model/modelstorage.py +++ b/trytond/model/modelstorage.py @@ -818,49 +818,6 @@ class ModelStorage(Model): return False return True - @classmethod - def check_recursion(cls, records, parent='parent', rec_name='rec_name'): - ''' - Function that checks if there is no recursion in the tree - composed with parent as parent field name. - ''' - parent_type = cls._fields[parent]._type - - if parent_type not in ('many2one', 'many2many', 'one2one'): - raise Exception( - 'Unsupported field type "%s" for field "%s" on "%s"' - % (parent_type, parent, cls.__name__)) - - visited = set() - - for record in records: - walked = set() - walker = getattr(record, parent) - while walker: - if parent_type == 'many2many': - for walk in walker: - walked.add(walk.id) - if walk.id == record.id: - parent_rec_name = ', '.join(getattr(r, rec_name) - for r in getattr(record, parent)) - cls.raise_user_error('recursion_error', { - 'rec_name': getattr(record, rec_name), - 'parent_rec_name': parent_rec_name, - }) - walker = list(chain(*(getattr(walk, parent) - for walk in walker if walk.id not in visited))) - else: - walked.add(walker.id) - if walker.id == record.id: - cls.raise_user_error('recursion_error', { - 'rec_name': getattr(record, rec_name), - 'parent_rec_name': getattr(getattr(record, - parent), rec_name) - }) - walker = (getattr(walker, parent) not in visited - and getattr(walker, parent)) - visited.update(walked) - @classmethod def _get_error_args(cls, field_name): pool = Pool() diff --git a/trytond/model/tree.py b/trytond/model/tree.py new file mode 100644 index 0000000000..fad30aecb4 --- /dev/null +++ b/trytond/model/tree.py @@ -0,0 +1,116 @@ +# This file is part of Tryton. The COPYRIGHT file at the top level of +# this repository contains the full copyright notices and license terms. +from itertools import chain + + +def tree(parent='parent', name='name', separator=None): + class TreeMixin(object): + + if separator: + @classmethod + def __setup__(cls): + super(TreeMixin, cls).__setup__() + field = getattr(cls, name) + field.domain = [ + field.domain, + (name, 'not like', '%' + separator + '%'), + ] + + def get_rec_name(self, _): + record, names = self, [] + while record: + names.append(getattr(record, name)) + record = getattr(record, parent) + return separator.join(reversed(names)) + + @classmethod + def search_rec_name(cls, _, clause): + if isinstance(clause[2], basestring): + values = list(reversed(clause[2].split(separator))) + else: + values = [[]] + for value in clause[2]: + if value is None: + values[0].append(value) + continue + for i, v in range(reversed(value.split(separator))): + while len(values) <= i: + values.append([]) + values[i].append(v) + domain = [] + field = name + for value in values: + domain.append((field, clause[1], value.strip())) + field = parent + '.' + field + if ((clause[1].endswith('like') + and not clause[2].replace( + '%%', '__').startswith('%')) + or not clause[1].endswith('like')): + if clause[1].startswith('not') or clause[1] == '!=': + operator = '!=' + domain.insert(0, 'OR') + else: + operator = '=' + top_parent = '.'.join((parent,) * len(values)) + domain.append((top_parent, operator, None)) + if (clause[1].endswith('like') + and clause[2].replace('%%', '__').endswith('%')): + ids = map(int, cls.search(domain, order=[])) + domain = [(parent, 'child_of', ids)] + return domain + + @classmethod + def validate(cls, records): + super(TreeMixin, cls).validate(records) + cls.check_recursion(records) + + @classmethod + def check_recursion(cls, records): + ''' + Function that checks if there is no recursion in the tree + composed with parent as parent field name. + ''' + if hasattr(super(TreeMixin, cls), 'check_recursion'): + super(TreeMixin, cls).check_recursion(records) + + parent_type = cls._fields[parent]._type + + if parent_type not in ('many2one', 'many2many', 'one2one'): + raise ValueError( + 'Unsupported field type "%s" for field "%s" on "%s"' + % (parent_type, parent, cls.__name__)) + + visited = set() + + for record in records: + walked = set() + walker = getattr(record, parent) + while walker: + if parent_type == 'many2many': + for walk in walker: + walked.add(walk.id) + if walk.id == record.id: + parent_name = ', '.join(getattr(r, name) + for r in getattr(record, parent)) + cls.raise_user_error('recursion_error', { + 'rec_name': getattr(record, name), + 'parent_rec_name': parent_name, + }) + walker = list(chain(*( + getattr(walk, parent) + for walk in walker + if walk.id not in visited))) + else: + walked.add(walker.id) + if walker.id == record.id: + parent_name = getattr( + getattr(record, parent), name) + cls.raise_user_error('recursion_error', { + 'rec_name': getattr(record, name), + 'parent_rec_name': parent_name + }) + walker = (getattr(walker, parent) not in visited + and getattr(walker, parent)) + visited.update(walked) + + return TreeMixin diff --git a/trytond/tests/__init__.py b/trytond/tests/__init__.py index 0e618b1f87..dcf500ac32 100644 --- a/trytond/tests/__init__.py +++ b/trytond/tests/__init__.py @@ -42,6 +42,7 @@ from . import field_many2many from . import field_one2one from . import multivalue from . import mixin +from . import tree def register(): @@ -124,6 +125,7 @@ def register(): field_one2one.register('tests') multivalue.register('tests') mixin.register('tests') + tree.register('tests') if pkg_resources is not None: entry_points = pkg_resources.iter_entry_points('trytond.tests') diff --git a/trytond/tests/mptt.py b/trytond/tests/mptt.py index b2f491d87b..777468ed90 100644 --- a/trytond/tests/mptt.py +++ b/trytond/tests/mptt.py @@ -4,14 +4,14 @@ from sql import Null from sql.conditionals import Case -from trytond.model import ModelView, ModelSQL, DeactivableMixin, fields +from trytond.model import ModelView, ModelSQL, DeactivableMixin, tree, fields __all__ = [ 'MPTT', ] -class MPTT(DeactivableMixin, ModelSQL, ModelView): +class MPTT(DeactivableMixin, tree(), ModelSQL, ModelView): 'Modified Preorder Tree Traversal' __name__ = 'test.mptt' name = fields.Char('Name', required=True) @@ -21,11 +21,6 @@ class MPTT(DeactivableMixin, ModelSQL, ModelView): right = fields.Integer('Right', required=True, select=True) childs = fields.One2Many('test.mptt', 'parent', 'Children') - @classmethod - def validate(cls, record): - super(MPTT, cls).validate(record) - cls.check_recursion(record) - @staticmethod def order_sequence(tables): table, _ = tables[None] diff --git a/trytond/tests/test_tree.py b/trytond/tests/test_tree.py new file mode 100644 index 0000000000..03f74221c3 --- /dev/null +++ b/trytond/tests/test_tree.py @@ -0,0 +1,191 @@ +# This file is part of Tryton. The COPYRIGHT file at the top level of +# this repository contains the full copyright notices and license terms. + +import unittest + +from trytond.exceptions import UserError +from trytond.tests.test_tryton import activate_module, with_transaction +from trytond.pool import Pool + + +class TreeMixinTestCase(unittest.TestCase): + "Test TreeMixin" + + @classmethod + def setUpClass(cls): + activate_module('tests') + + @with_transaction() + def test_name_domain(self): + "Test name domain" + pool = Pool() + Tree = pool.get('test.tree') + + record = Tree(name="foo / bar") + + with self.assertRaises(UserError): + record.save() + + @with_transaction() + def test_rec_name(self): + "Test rec_name" + pool = Pool() + Tree = pool.get('test.tree') + + parent = Tree(name="parent") + parent.save() + record = Tree(name="record", parent=parent) + record.save() + + self.assertEqual(record.rec_name, "parent / record") + + @with_transaction() + def test_search_rec_name_equals(self): + "Test search_rec_name equals" + pool = Pool() + Tree = pool.get('test.tree') + + parent = Tree(name="parent") + parent.save() + record = Tree(name="record", parent=parent) + record.save() + + records = Tree.search([('rec_name', '=', 'parent / record')]) + + self.assertEqual(records, [record]) + + @with_transaction() + def test_search_rec_name_equals_toplevel(self): + "Test search_rec_name equals top-level" + pool = Pool() + Tree = pool.get('test.tree') + + parent = Tree(name="parent") + parent.save() + record = Tree(name="record", parent=parent) + record.save() + + records = Tree.search([('rec_name', '=', 'parent')]) + + self.assertEqual(records, [parent]) + + @with_transaction() + def test_search_rec_name_non_equals(self): + "Test search_rec_name non equals" + pool = Pool() + Tree = pool.get('test.tree') + + parent = Tree(name="parent") + parent.save() + record = Tree(name="record", parent=parent) + record.save() + + records = Tree.search([('rec_name', '!=', 'parent / record')]) + + self.assertEqual(records, [parent]) + + @with_transaction() + def test_search_rec_name_non_equals_toplevel(self): + "Test search_rec_name non equals top-level" + pool = Pool() + Tree = pool.get('test.tree') + + parent = Tree(name="parent") + parent.save() + record = Tree(name="record", parent=parent) + record.save() + + records = Tree.search([('rec_name', '!=', 'parent')]) + + self.assertEqual(records, [record]) + + @with_transaction() + def test_search_rec_name_like(self): + "Test search_rec_name like" + pool = Pool() + Tree = pool.get('test.tree') + + parent = Tree(name="parent") + parent.save() + record = Tree(name="record", parent=parent) + record.save() + child = Tree(name="child", parent=record) + child.save() + + records = Tree.search([('rec_name', 'like', '%record%')]) + + self.assertEqual(records, [record, child]) + + @with_transaction() + def test_search_rec_name_like_toplevel(self): + "Test search_rec_name like top-level" + pool = Pool() + Tree = pool.get('test.tree') + + parent = Tree(name="parent") + parent.save() + record = Tree(name="record", parent=parent) + record.save() + child = Tree(name="child", parent=record) + child.save() + + records = Tree.search([('rec_name', 'like', 'parent / record%')]) + + self.assertEqual(records, [record, child]) + + @with_transaction() + def test_search_rec_name_like_lowlevel(self): + "Test search_rec_name like low-level" + pool = Pool() + Tree = pool.get('test.tree') + + parent = Tree(name="parent") + parent.save() + record = Tree(name="record", parent=parent) + record.save() + child = Tree(name="child", parent=record) + child.save() + + records = Tree.search([('rec_name', 'like', '%record')]) + + self.assertEqual(records, [record]) + + @with_transaction() + def test_check_recursion(self): + "Test check_recursion" + pool = Pool() + Tree = pool.get('test.tree') + + parent = Tree(name="parent") + parent.save() + record = Tree(name="record", parent=parent) + record.save() + child = Tree(name="child", parent=record) + child.save() + + with self.assertRaises(UserError): + parent.parent = child + parent.save() + + @with_transaction() + def test_check_recursion_polytree(self): + "Test check_recursion on polytree" + pool = Pool() + Polytree = pool.get('test.polytree') + + parent1 = Polytree(name="parent1") + parent1.save() + parent2 = Polytree(name="parent2") + parent2.save() + record = Polytree(name="record", parents=[parent1, parent2]) + record.save() + child = Polytree(name="child", parents=[record]) + child.save() + + with self.assertRaises(UserError): + parent1.parents = [child] + parent1.save() + + +def suite(): + return unittest.TestLoader().loadTestsFromTestCase(TreeMixinTestCase) diff --git a/trytond/tests/tree.py b/trytond/tests/tree.py new file mode 100644 index 0000000000..fb4205755a --- /dev/null +++ b/trytond/tests/tree.py @@ -0,0 +1,35 @@ +# This file is part of Tryton. The COPYRIGHT file at the top level of +# this repository contains the full copyright notices and license terms. + +from trytond.model import ModelSQL, tree, fields +from trytond.pool import Pool + + +class Tree(tree(separator=' / '), ModelSQL): + "Tree" + __name__ = 'test.tree' + name = fields.Char("Name") + parent = fields.Many2One('test.tree', "Parent") + + +class Polytree(tree(parent='parents'), ModelSQL): + "PolyTree" + __name__ = 'test.polytree' + name = fields.Char("Name") + parents = fields.Many2Many( + 'test.polytree.edge', 'parent', 'child', "Parents") + + +class PolytreeEdge(ModelSQL): + "Polytree Edge" + __name__ = 'test.polytree.edge' + parent = fields.Many2One('test.polytree', "Parent") + child = fields.Many2One('test.polytree', "Child") + + +def register(module): + Pool.register( + Tree, + Polytree, + PolytreeEdge, + module=module, type_='model')