From 6cd2e51aa35f708667aa4aebe7841cb9cc272175 Mon Sep 17 00:00:00 2001 From: Emmanuelle Delescolle Date: Wed, 16 May 2012 15:48:00 +0200 Subject: [PATCH] Multi-level inherits search fix Trytond now looks at the whole inherits hierarchy for fields when creating search domain on ModelSQL objects instead of just looking at the direct parent. issue1860 review369001 --- trytond/model/modelsql.py | 83 ++++++++++------ trytond/test/model.py | 52 ++++++++++ trytond/tests/test_model_inherits.py | 139 +++++++++++++++++++++++++++ trytond/tests/test_tryton.py | 2 + 4 files changed, 248 insertions(+), 28 deletions(-) create mode 100644 trytond/tests/test_model_inherits.py diff --git a/trytond/model/modelsql.py b/trytond/model/modelsql.py index 250eacfd6c..d9599f23f3 100644 --- a/trytond/model/modelsql.py +++ b/trytond/model/modelsql.py @@ -1348,6 +1348,40 @@ class ModelSQL(ModelStorage): domain = domain[:] cursor = Transaction().cursor + def column_get(model, field_name): + field = model._columns.get(field_name) + if not field: + if not field_name in model._inherit_fields: + raise Exception('ValidateError', 'Field "%s" doesn\'t ' \ + 'exist on "%s"' % (field_name, model._name)) + parent_model = pool.get(model._inherit_fields[field_name][0]) + field = column_get(parent_model, field_name) + return field + + def get_model_for_field(model, field): + if field in model._columns: + return model + else: + parent_model = pool.get(model._inherit_fields[field][0]) + return get_model_for_field(parent_model, field) + + def get_inherits_join(model, field): + if field in model._columns: + return [] + itable = pool.get(model._inherit_fields[field][0]) + table_query = '' + table_arg = [] + if itable.table_query(): + table_query, table_args = model.table_query() + table_query = '(' + table_query + ') AS ' + table_join = 'LEFT JOIN ' + table_query + \ + '"' + itable._table + '" ON ' \ + '"%s".id = "%s"."%s"' % (itable._table, model._table, + model._inherits[itable._name]) + inherits = [(table_join, table_arg)] + inherits.extend(get_inherits_join(itable, field)) + return inherits + for arg in domain: if arg[1] not in OPERATORS: raise Exception('ValidateError', 'Argument "%s" not supported' @@ -1358,20 +1392,13 @@ class ModelSQL(ModelStorage): table = self fargs = domain[i][0].split('.', 1) if fargs[0] in self._inherit_fields: - itable = pool.get(self._inherit_fields[fargs[0]][0]) - table_query = '' - table_arg = [] - if itable.table_query(): - table_query, table_args = self.table_query() - table_query = '(' + table_query + ') AS ' - table_join = 'LEFT JOIN ' + table_query + \ - '"' + itable._table + '" ON ' \ - '"%s".id = "%s"."%s"' % (itable._table, self._table, - self._inherits[itable._name]) - if table_join not in tables: - tables.append(table_join) - tables_args.extend(table_arg) - field = table._columns.get(fargs[0], False) + inherit_joins = get_inherits_join(self, fargs[0]) + for (table_join, table_arg) in inherit_joins: + if table_join not in tables: + tables.append(table_join) + tables_args.extend(table_arg) + + field = column_get(self, fargs[0]) if not field: if not fargs[0] in self._inherit_fields: raise Exception('ValidateError', 'Field "%s" doesn\'t ' \ @@ -1698,9 +1725,10 @@ class ModelSQL(ModelStorage): qu1, qu2 = [], [] for arg in domain: - table = self - if len(arg) > 3: + table = get_model_for_field(self, arg[0]) + if table == self and len(arg) > 3: table = arg[3] + column = column_get(table, arg[0]) if arg[1] in ('inselect', 'notinselect'): clause = 'IN' if arg[1] == 'notinselect': @@ -1711,7 +1739,7 @@ class ModelSQL(ModelStorage): elif arg[1] in ('in', 'not in'): if len(arg[2]) > 0: todel = [] - if table._columns[arg[0]]._type != 'boolean': + if column._type != 'boolean': for xitem in range(len(arg[2])): if (arg[2][xitem] is False or arg[2][xitem] is None): @@ -1719,7 +1747,7 @@ class ModelSQL(ModelStorage): arg2 = arg[2][:] for xitem in todel[::-1]: del arg2[xitem] - arg2 = [FIELDS[table._columns[arg[0]]._type].sql_format(x) + arg2 = [FIELDS[column._type].sql_format(x) for x in arg2] if len(arg2): if reduce(lambda x, y: (x @@ -1738,7 +1766,7 @@ class ModelSQL(ModelStorage): ('%s',) * len(arg2)))) qu2 += arg2 if todel: - if table._columns[arg[0]]._type == 'boolean': + if column._type == 'boolean': if arg[1] == 'in': qu1[-1] = '(' + qu1[-1] + ' OR ' \ '"%s"."%s" = %%s)' % \ @@ -1759,7 +1787,7 @@ class ModelSQL(ModelStorage): '"%s"."%s" IS NOT NULL)' % \ (table._table, arg[0]) elif todel: - if table._columns[arg[0]]._type == 'boolean': + if column._type == 'boolean': if arg[1] == 'in': qu1.append('("%s"."%s" = %%s)' % \ (table._table, arg[0])) @@ -1784,7 +1812,7 @@ class ModelSQL(ModelStorage): qu2.append(True) else: if (arg[2] is False or arg[2] is None) and (arg[1] == '='): - if table._columns[arg[0]]._type == 'boolean': + if column._type == 'boolean': qu1.append('("%s"."%s" = %%s)' % \ (table._table, arg[0])) qu2.append(False) @@ -1798,7 +1826,7 @@ class ModelSQL(ModelStorage): if arg[0] == 'id': qu1.append('("%s"."%s" %s %%s)' % \ (table._table, arg[0], arg[1])) - qu2.append(FIELDS[table._columns[arg[0]]._type].\ + qu2.append(FIELDS[column._type].\ sql_format(arg[2])) else: add_null = False @@ -1808,21 +1836,20 @@ class ModelSQL(ModelStorage): add_null = True else: qu2.append(FIELDS[ - table._columns[arg[0]]._type + column._type ].sql_format(arg[2])) elif arg[1] in ('not like', 'not ilike'): if not arg[2]: qu2.append('') else: qu2.append(FIELDS[ - table._columns[arg[0]]._type + column._type ].sql_format(arg[2])) add_null = True else: - if arg[0] in table._columns: - qu2.append(FIELDS[ - table._columns[arg[0]]._type - ].sql_format(arg[2])) + qu2.append(FIELDS[ + column._type + ].sql_format(arg[2])) qu1.append('("%s"."%s" %s %%s)' % (table._table, arg[0], arg[1])) if add_null: diff --git a/trytond/test/model.py b/trytond/test/model.py index 24b56babd9..44318f3c99 100644 --- a/trytond/test/model.py +++ b/trytond/test/model.py @@ -23,3 +23,55 @@ class URLObject(ModelSQL): URLObject() + + +class ModelInherits(ModelSQL): + _name = 'test.model_inherits' + + name = fields.Char('Name') + +ModelInherits() + + +class ModelInheritsSubModel(ModelSQL): + _name = 'test.submodel' + _inherits = {'test.model_inherits': 'parent_model'} + + subfield = fields.Char('SubField') + parent_model = fields.Many2One('test.model_inherits', 'Parent model', + required=True) + +ModelInheritsSubModel() + + +class ModelInheritsSubSubModel(ModelSQL): + _name = 'test.subsubmodel' + _inherits = {'test.submodel': 'parent_model'} + + subsubfield = fields.Char('SubField') + parent_model = fields.Many2One('test.submodel', 'Parent model', + required=True) + +ModelInheritsSubSubModel() + + +class ModelInheritsSubSubSubModel(ModelSQL): + _name = 'test.subsubsubmodel' + _inherits = {'test.subsubmodel': 'parent_model'} + + subsubsubfield = fields.Char('SubField') + parent_model = fields.Many2One('test.subsubmodel', 'Parent model', + required=True) + +ModelInheritsSubSubSubModel() + + +class ModelInheritsOverriddenFieldModel(ModelSQL): + _name = 'test.overriddeninheritedfieldmodel' + _inherits = {'test.subsubmodel': 'parent_model'} + + subfield = fields.Integer('Overridden field') + parent_model = fields.Many2One('test.subsubmodel', 'Parent model', + required=True) + +ModelInheritsOverriddenFieldModel() diff --git a/trytond/tests/test_model_inherits.py b/trytond/tests/test_model_inherits.py new file mode 100644 index 0000000000..3f9a46dd82 --- /dev/null +++ b/trytond/tests/test_model_inherits.py @@ -0,0 +1,139 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- +#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.tests.test_tryton import POOL, DB_NAME, USER, CONTEXT, \ + install_module +from trytond.transaction import Transaction + + +class ModelInheritsTestCase(unittest.TestCase): + ''' + Test ModelInheritSearch + ''' + + def setUp(self): + install_module('test') + self.parent = POOL.get('test.model_inherits') + self.sub = POOL.get('test.submodel') + self.subsub = POOL.get('test.subsubmodel') + self.subsubsub = POOL.get('test.subsubsubmodel') + self.overridden = POOL.get('test.overriddeninheritedfieldmodel') + + def create_data(self): + parent_id = self.parent.create({'name': 'parent'}) + sub_id = self.sub.create({ + 'name': 'sub', + 'subfield': 'level1', + }) + subsub_id = self.subsub.create({ + 'name': 'subsub', + 'subfield': 'level2', + 'subsubfield': 'ssfcontent', + }) + subsubsub_id = self.subsubsub.create({ + 'name': 'subsubsub', + 'subfield': 'level3', + 'subsubfield': 'ssfcontent2', + 'subsubsubfield': 'sssfcontent', + }) + overridden_id = self.overridden.create({ + 'name': 'overridden', + 'subfield': 3, + }) + return (parent_id, sub_id, subsub_id, subsubsub_id, overridden_id) + + def test0010selfsearch(self): + ''' + Test searching on own field + ''' + with Transaction().start(DB_NAME, USER, + context=CONTEXT) as transaction: + + (_parent_id, _sub_id, _subsub_id, subsubsub_id, overridden_id) = \ + self.create_data() + + ids = self.subsubsub.search([ + ('subsubsubfield', '=', 'sssfcontent'), + ]) + self.assertEqual(ids, [subsubsub_id]) + + transaction.cursor.rollback() + + def test0020parentsearch(self): + ''' + Test searching on parent field + ''' + with Transaction().start(DB_NAME, USER, + context=CONTEXT) as transaction: + + (_parent_id, _sub_id, _subsub_id, subsubsub_id, overridden_id) = \ + self.create_data() + + ids = self.subsubsub.search([('subsubfield', '=', 'ssfcontent2')]) + self.assertEqual(ids, [subsubsub_id]) + + ids = self.subsubsub.search([('subsubfield', '=', 'ssfcontent')]) + self.assertEqual(ids, []) + + transaction.cursor.rollback() + + def test0030granparentsearch(self): + ''' + Test searching on granparent field + ''' + with Transaction().start(DB_NAME, USER, + context=CONTEXT) as transaction: + + (_parent_id, _sub_id, _subsub_id, subsubsub_id, overridden_id) = \ + self.create_data() + + ids = self.subsubsub.search([('subfield', '=', 'level3')]) + self.assertEqual(ids, [subsubsub_id]) + + ids = self.subsubsub.search([('subfield', '=', 'level1')]) + self.assertEqual(ids, []) + + transaction.cursor.rollback() + + def test0040greatgranparentsearch(self): + ''' + Test searching on greatgranparent field + ''' + with Transaction().start(DB_NAME, USER, + context=CONTEXT) as transaction: + + (_parent_id, _sub_id, _subsub_id, subsubsub_id, overridden_id) = \ + self.create_data() + + ids = self.subsubsub.search([('name', '=', 'subsubsub')]) + self.assertEqual(ids, [subsubsub_id]) + + ids = self.subsubsub.search([('name', '=', 'parent')]) + self.assertEqual(ids, []) + + transaction.cursor.rollback() + + def test0050overriddenfieldsearch(self): + ''' + Test searching on greatgranparent field + ''' + with Transaction().start(DB_NAME, USER, + context=CONTEXT) as transaction: + + (_parent_id, _sub_id, _subsub_id, subsubsub_id, overridden_id) = \ + self.create_data() + + ids = self.overridden.search([('subfield', '=', 3)]) + self.assertEqual(ids, [overridden_id]) + + transaction.cursor.rollback() + + +def suite(): + return unittest.TestLoader().loadTestsFromTestCase(ModelInheritsTestCase) + +if __name__ == '__main__': + suite = suite() + unittest.TextTestRunner(verbosity=2).run(suite) diff --git a/trytond/tests/test_tryton.py b/trytond/tests/test_tryton.py index 307230ad71..d63526d16e 100644 --- a/trytond/tests/test_tryton.py +++ b/trytond/tests/test_tryton.py @@ -251,6 +251,8 @@ def all_suite(): suite_.addTests(test_mixins.suite()) import trytond.tests.test_wizard as test_wizard suite_.addTests(test_wizard.suite()) + import trytond.tests.test_model_inherits as test_inherits + suite_.addTests(test_inherits.suite()) return suite_