From 3fc5996c3e46225dc8b83202f714744c6be8130e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Voet=20=28ryv=29?= Date: Wed, 9 Feb 2022 14:42:36 +0000 Subject: [PATCH] [REF] core: refactor the `filtered_domain` method of `BaseModel` The method filtered_domain() didn't conserve the order of `self`, which may be confusing, and a subtle source of bugs. It now preserves the order from `self`. The processing of `child_of`/`parent_of` making a search with the default order of the model, which may be slow. Add documentation. Part-of: odoo/odoo#83687 --- odoo/addons/base/tests/test_expression.py | 19 ++- odoo/models.py | 137 ++++++++++++---------- 2 files changed, 89 insertions(+), 67 deletions(-) diff --git a/odoo/addons/base/tests/test_expression.py b/odoo/addons/base/tests/test_expression.py index 3d814acb5e9..69a764521e8 100644 --- a/odoo/addons/base/tests/test_expression.py +++ b/odoo/addons/base/tests/test_expression.py @@ -22,11 +22,10 @@ class TestExpression(SavepointCaseWithUserDemo): cls._load_partners_set() cls.env['res.currency'].with_context({'active_test': False}).search([('name', 'in', ['EUR', 'USD'])]).write({'active': True}) - def _search(self, obj, domain, init_domain=[]): - sql = obj.search(domain) - allobj = obj.search(init_domain) - fil = allobj.filtered_domain(domain) - self.assertEqual(sql, fil, "filtered_domain do not match SQL search for domain: "+str(domain)) + def _search(self, model, domain, init_domain=None): + sql = model.search(domain, order="id") + fil = model.search(init_domain or [], order="id").filtered_domain(domain) + self.assertEqual(sql._ids, fil._ids, f"filtered_domain do not match SQL search for domain: {domain}") return sql def test_00_in_not_in_m2m(self): @@ -873,6 +872,16 @@ class TestExpression(SavepointCaseWithUserDemo): countries = countries.browse(reversed(countries._ids)) self.assertEqual(countries.filtered_domain(domain)._ids, countries._ids) + def test_filtered_domain_order2(self): + countries = self.env['res.country'].search([]) + # match the first two countries, in order + expected = countries[:2] + id1, id2 = expected._ids + domain = ['|', ('id', '=', id1), ('id', '=', id2)] + self.assertEqual(countries.filtered_domain(domain)._ids, expected._ids) + domain = ['|', ('id', '=', id2), ('id', '=', id1)] + self.assertEqual(countries.filtered_domain(domain)._ids, expected._ids) + class TestExpression2(TransactionCase): diff --git a/odoo/models.py b/odoo/models.py index ea9dbbc5c58..a98a6511ef3 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -5520,28 +5520,36 @@ Fields: return self.browse([rec.id for rec in self if func(rec)]) def filtered_domain(self, domain): - if not domain: return self - result = [] - for d in reversed(domain): - if d == '|': - result.append(result.pop() | result.pop()) - elif d == '!': - result.append(self - result.pop()) - elif d == '&': - result.append(result.pop() & result.pop()) - elif d == expression.TRUE_LEAF: - result.append(self) - elif d == expression.FALSE_LEAF: - result.append(self.browse()) + """Return the records in ``self`` satisfying the domain and keeping the same order. + + :param domain: :ref:`A search domain `. + """ + if not domain or not self: + return self + + stack = [] + for leaf in reversed(domain): + if leaf == '|': + stack.append(stack.pop() | stack.pop()) + elif leaf == '!': + stack.append(set(self._ids) - stack.pop()) + elif leaf == '&': + stack.append(stack.pop() & stack.pop()) + elif leaf == expression.TRUE_LEAF: + stack.append(set(self._ids)) + elif leaf == expression.FALSE_LEAF: + stack.append(set()) else: - (key, comparator, value) = d + (key, comparator, value) = leaf if comparator in ('child_of', 'parent_of'): - result.append(self.search([('id', 'in', self.ids), d])) + stack.append(set(self.search([('id', 'in', self.ids), leaf], order='id')._ids)) continue + if key.endswith('.id'): key = key[:-3] if key == 'id': key = '' + # determine the field with the final type for values field = None if key: @@ -5549,76 +5557,81 @@ Fields: for fname in key.split('.'): field = model._fields[fname] model = model[fname] + if comparator in ('like', 'ilike', '=like', '=ilike', 'not ilike', 'not like'): value_esc = value.replace('_', '?').replace('%', '*').replace('[', '?') - records_ids = OrderedSet() - for rec in self: - data = rec.mapped(key) + if comparator in ('in', 'not in'): + if isinstance(value, (list, tuple)): + value = set(value) + else: + value = (value,) + if field and field.type in ('date', 'datetime'): + value = {Datetime.to_datetime(v) for v in value} + elif field and field.type in ('date', 'datetime'): + value = Datetime.to_datetime(value) + + matching_ids = set() + for record in self: + data = record.mapped(key) if isinstance(data, BaseModel): v = value - if (isinstance(value, list) or isinstance(value, tuple)) and len(value): - v = value[0] + if isinstance(value, (list, tuple, set)) and value: + v = next(iter(value)) if isinstance(v, str): data = data.mapped('display_name') else: data = data and data.ids or [False] elif field and field.type in ('date', 'datetime'): - # convert all date and datetime values to datetime - normalize = Datetime.to_datetime - if isinstance(value, (list, tuple)): - value = [normalize(v) for v in value] - else: - value = normalize(value) - data = [normalize(d) for d in data] - if comparator in ('in', 'not in'): - if not (isinstance(value, list) or isinstance(value, tuple)): - value = [value] + data = [Datetime.to_datetime(d) for d in data] if comparator == '=': ok = value in data - elif comparator == 'in': - ok = any(map(lambda x: x in data, value)) - elif comparator == '<': - ok = any(map(lambda x: x is not None and x < value, data)) - elif comparator == '>': - ok = any(map(lambda x: x is not None and x > value, data)) - elif comparator == '<=': - ok = any(map(lambda x: x is not None and x <= value, data)) - elif comparator == '>=': - ok = any(map(lambda x: x is not None and x >= value, data)) elif comparator in ('!=', '<>'): ok = value not in data + elif comparator == '=?': + ok = not value or (value in data) + elif comparator == 'in': + ok = value and any(x in value for x in data) elif comparator == 'not in': - ok = all(map(lambda x: x not in data, value)) - elif comparator == 'not ilike': - data = [(x or "") for x in data] - ok = all(map(lambda x: value.lower() not in x.lower(), data)) + ok = not (value and any(x in value for x in data)) + elif comparator == '<': + ok = any(x is not None and x < value for x in data) + elif comparator == '>': + ok = any(x is not None and x > value for x in data) + elif comparator == '<=': + ok = any(x is not None and x <= value for x in data) + elif comparator == '>=': + ok = any(x is not None and x >= value for x in data) elif comparator == 'ilike': data = [(x or "").lower() for x in data] - ok = bool(fnmatch.filter(data, '*'+(value_esc or '').lower()+'*')) - elif comparator == 'not like': - data = [(x or "") for x in data] - ok = all(map(lambda x: value not in x, data)) + ok = fnmatch.filter(data, '*' + (value_esc or '').lower() + '*') + elif comparator == 'not ilike': + value = value.lower() + ok = not any(value in (x or "").lower() for x in data) elif comparator == 'like': data = [(x or "") for x in data] - ok = bool(fnmatch.filter(data, value and '*'+value_esc+'*')) - elif comparator == '=?': - ok = (value in data) or not value - elif comparator in ('=like'): + ok = fnmatch.filter(data, value and '*' + value_esc + '*') + elif comparator == 'not like': + ok = not any(value in (x or "") for x in data) + elif comparator == '=like': data = [(x or "") for x in data] - ok = bool(fnmatch.filter(data, value_esc)) - elif comparator in ('=ilike'): + ok = fnmatch.filter(data, value_esc) + elif comparator == '=ilike': data = [(x or "").lower() for x in data] - ok = bool(fnmatch.filter(data, value and value_esc.lower())) + ok = fnmatch.filter(data, value and value_esc.lower()) else: - raise ValueError - if ok: - records_ids.add(rec.id) - result.append(self.browse(records_ids)) - while len(result)>1: - result.append(result.pop() & result.pop()) - return result[0] + raise ValueError(f"Invalid term domain '{leaf}', operator '{comparator}' doesn't exist.") + if ok: + matching_ids.add(record.id) + + stack.append(matching_ids) + + while len(stack) > 1: + stack.append(stack.pop() & stack.pop()) + + [result_ids] = stack + return self.browse(id_ for id_ in self._ids if id_ in result_ids) def sorted(self, key=None, reverse=False): """Return the recordset ``self`` ordered by ``key``.