From fd1c982a40bfd661de9fdd92dccfbf0417bde1fc Mon Sep 17 00:00:00 2001 From: Gorash Date: Wed, 20 Apr 2022 07:17:04 +0000 Subject: [PATCH] [IMP] core: handle recordset comparison with lazy() recordset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The base model is modified in order to be able to make comparisons between recordsets contained in lazy values. The isinstance method will always return an error, so we consider that we receive a recordset and try to access the `_name` and `_ids`. If an error is triggered, it was not a recordset. This way of doing it involves little change in performance (improvement when it is good and decrease when there is an error) because we do not test if it is a recordset before comparing it. closes odoo/odoo#89141 Signed-off-by: Rémy Voet --- odoo/addons/base/tests/test_api.py | 98 +++++++++++++++++++++--- odoo/models.py | 117 ++++++++++++++++------------- 2 files changed, 152 insertions(+), 63 deletions(-) diff --git a/odoo/addons/base/tests/test_api.py b/odoo/addons/base/tests/test_api.py index e521171ff35..4db957b9399 100644 --- a/odoo/addons/base/tests/test_api.py +++ b/odoo/addons/base/tests/test_api.py @@ -3,7 +3,7 @@ from odoo import api, models, Command from odoo.addons.base.tests.common import SavepointCaseWithUserDemo -from odoo.tools import mute_logger, unique +from odoo.tools import mute_logger, unique, lazy from odoo.exceptions import AccessError @@ -453,15 +453,33 @@ class TestAPI(SavepointCaseWithUserDemo): @mute_logger('odoo.models') def test_80_contains(self): """ Test membership on recordset. """ - p1 = self.env['res.partner'].search([('name', 'ilike', 'a'), ('id', 'in', self.partners.ids)], limit=1).ensure_one() - ps = self.env['res.partner'].search([('name', 'ilike', 'a'), ('id', 'in', self.partners.ids)]) + p1 = self.partners[0] + ps = self.partners self.assertTrue(p1 in ps) + with self.assertRaisesRegex(TypeError, r"unsupported operand types in: 42 in res\.partner.*"): + 42 in ps + with self.assertRaisesRegex(TypeError, r"inconsistent models in: ir\.ui\.menu.* in res\.partner.*"): + self.env['ir.ui.menu'] in ps + + @mute_logger('odoo.models') + def test_80_lazy_contains(self): + """ Test membership on recordset. """ + p1 = lazy(lambda: self.partners[0]) + ps = lazy(lambda: self.partners) + self.assertTrue(p1 in ps) + + with self.assertRaisesRegex(TypeError, r"unsupported operand types in: 42 in res\.partner.*"): + lazy(lambda: 42) in ps + with self.assertRaisesRegex(TypeError, r"inconsistent models in: ir\.ui\.menu.* in res\.partner.*"): + lazy(lambda: self.env['ir.ui.menu']) in ps + @mute_logger('odoo.models') def test_80_set_operations(self): """ Check set operations on recordsets. """ pa = self.env['res.partner'].search([('name', 'ilike', 'a'), ('id', 'in', self.partners.ids)]) pb = self.env['res.partner'].search([('name', 'ilike', 'b'), ('id', 'in', self.partners.ids)]) + self.assertTrue(pa) self.assertTrue(pb) self.assertTrue(set(pa) & set(pb)) @@ -493,22 +511,80 @@ class TestAPI(SavepointCaseWithUserDemo): self.assertNotEqual(ps._name, ms._name) self.assertNotEqual(ps, ms) + with self.assertRaisesRegex(TypeError, r"unsupported operand types in: res\.partner.* \+ 'string'"): + ps + 'string' + with self.assertRaisesRegex(TypeError, r"inconsistent models in: res\.partner.* \+ ir\.ui\.menu.*"): + ps + ms + with self.assertRaisesRegex(TypeError, r"inconsistent models in: res\.partner.* - ir\.ui\.menu.*"): + ps - ms + with self.assertRaisesRegex(TypeError, r"inconsistent models in: res\.partner.* & ir\.ui\.menu.*"): + ps & ms + with self.assertRaisesRegex(TypeError, r"inconsistent models in: res\.partner.* \| ir\.ui\.menu.*"): + ps | ms with self.assertRaises(TypeError): - res = ps + ms + ps < ms with self.assertRaises(TypeError): - res = ps - ms + ps <= ms with self.assertRaises(TypeError): - res = ps & ms + ps > ms with self.assertRaises(TypeError): - res = ps | ms + ps >= ms + + @mute_logger('odoo.models') + def test_80_lazy_set_operations(self): + """ Check set operations on recordsets. """ + pa = lazy(lambda: self.env['res.partner'].search([('name', 'ilike', 'a'), ('id', 'in', self.partners.ids)])) + pb = lazy(lambda: self.env['res.partner'].search([('name', 'ilike', 'b'), ('id', 'in', self.partners.ids)])) + + self.assertTrue(pa) + self.assertTrue(pb) + self.assertTrue(set(pa) & set(pb)) + + concat = pa + pb + self.assertEqual(list(concat), list(pa) + list(pb)) + self.assertEqual(len(concat), len(pa) + len(pb)) + + difference = pa - pb + self.assertEqual(len(difference), len(set(difference))) + self.assertEqual(set(difference), set(pa) - set(pb)) + self.assertLessEqual(difference, pa) + + intersection = pa & pb + self.assertEqual(len(intersection), len(set(intersection))) + self.assertEqual(set(intersection), set(pa) & set(pb)) + self.assertLessEqual(intersection, pa) + self.assertLessEqual(intersection, pb) + + union = pa | pb + self.assertEqual(len(union), len(set(union))) + self.assertEqual(set(union), set(pa) | set(pb)) + self.assertGreaterEqual(union, pa) + self.assertGreaterEqual(union, pb) + + # one cannot mix different models with set operations + ps = pa + ms = lazy(lambda: self.env['ir.ui.menu'].search([])) + self.assertNotEqual(ps._name, ms._name) + self.assertNotEqual(ps, ms) + + with self.assertRaisesRegex(TypeError, r"unsupported operand types in: res\.partner.* \+ 'string'"): + ps + 'string' + with self.assertRaisesRegex(TypeError, r"inconsistent models in: res\.partner.* \+ ir\.ui\.menu.*"): + ps + ms + with self.assertRaisesRegex(TypeError, r"inconsistent models in: res\.partner.* - ir\.ui\.menu.*"): + ps - ms + with self.assertRaisesRegex(TypeError, r"inconsistent models in: res\.partner.* & ir\.ui\.menu.*"): + ps & ms + with self.assertRaisesRegex(TypeError, r"inconsistent models in: res\.partner.* \| ir\.ui\.menu.*"): + ps | ms with self.assertRaises(TypeError): - res = ps < ms + ps < ms with self.assertRaises(TypeError): - res = ps <= ms + ps <= ms with self.assertRaises(TypeError): - res = ps > ms + ps > ms with self.assertRaises(TypeError): - res = ps >= ms + ps >= ms @mute_logger('odoo.models') def test_80_filter(self): diff --git a/odoo/models.py b/odoo/models.py index 60720e79c73..4c8459739e1 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -5887,14 +5887,14 @@ Fields: any(item == record for record in self) """ - if isinstance(item, BaseModel) and self._name == item._name: - return len(item) == 1 and item.id in self._ids - elif isinstance(item, str): - return item in self._fields - elif isinstance(item, BaseModel): - raise TypeError(f"cannot compare different models: '{self._name}()' and '{item._name}()'") - else: - raise TypeError(f"unsupported operand type(s) for \"in\": '{self._name}()' and '{type(item)}'") + try: + if self._name == item._name: + return len(item) == 1 and item.id in self._ids + raise TypeError(f"inconsistent models in: {item} in {self}") + except AttributeError: + if isinstance(item, str): + return item in self._fields + raise TypeError(f"unsupported operand types in: {item!r} in {self}") def __add__(self, other): """ Return the concatenation of two recordsets. """ @@ -5906,37 +5906,37 @@ Fields: """ ids = list(self._ids) for arg in args: - if isinstance(arg, BaseModel) and arg._name == self._name: + try: + if arg._name != self._name: + raise TypeError(f"inconsistent models in: {self} + {arg}") ids.extend(arg._ids) - elif isinstance(arg, BaseModel): - raise TypeError(f"cannot concat different models: '{self._name}()' and '{arg._name}()'") - else: - raise TypeError(f"unsupported operand type(s) for \"concat\": '{self._name}()' and '{type(arg)}'") + except AttributeError: + raise TypeError(f"unsupported operand types in: {self} + {arg!r}") return self.browse(ids) def __sub__(self, other): """ Return the recordset of all the records in ``self`` that are not in ``other``. Note that recordset order is preserved. """ - if isinstance(other, BaseModel) and self._name == other._name: + try: + if self._name != other._name: + raise TypeError(f"inconsistent models in: {self} - {other}") other_ids = set(other._ids) - elif isinstance(other, BaseModel): - raise TypeError(f"cannot substract different models: '{self._name}()' and '{other._name}()'") - else: - raise TypeError(f"unsupported operand type(s) for \"-\": '{self._name}()' and '{type(other)}'") - return self.browse([id for id in self._ids if id not in other_ids]) + return self.browse([id for id in self._ids if id not in other_ids]) + except AttributeError: + raise TypeError(f"unsupported operand types in: {self} - {other!r}") def __and__(self, other): """ Return the intersection of two recordsets. Note that first occurrence order is preserved. """ - if isinstance(other, BaseModel) and self._name == other._name: + try: + if self._name != other._name: + raise TypeError(f"inconsistent models in: {self} & {other}") other_ids = set(other._ids) - elif isinstance(other, BaseModel): - raise TypeError(f"cannot add different models: '{self._name}()' and '{other._name}()'") - else: - raise TypeError(f"unsupported operand type(s) for \"+\": '{self._name}()' and '{type(other)}'") - return self.browse(OrderedSet(id for id in self._ids if id in other_ids)) + return self.browse(OrderedSet(id for id in self._ids if id in other_ids)) + except AttributeError: + raise TypeError(f"unsupported operand types in: {self} & {other!r}") def __or__(self, other): """ Return the union of two recordsets. @@ -5950,50 +5950,63 @@ Fields: """ ids = list(self._ids) for arg in args: - if isinstance(arg, BaseModel) and self._name == arg._name: + try: + if arg._name != self._name: + raise TypeError(f"inconsistent models in: {self} | {arg}") ids.extend(arg._ids) - elif isinstance(arg, BaseModel): - raise TypeError(f"cannot union different models: '{self._name}()' and '{arg._name}()'") - else: - raise TypeError(f"unsupported operand type(s) for \"union\": '{self._name}()' and '{type(arg)}'") + except AttributeError: + raise TypeError(f"unsupported operand types in: {self} | {arg!r}") return self.browse(OrderedSet(ids)) def __eq__(self, other): """ Test whether two recordsets are equivalent (up to reordering). """ - if not isinstance(other, BaseModel): + try: + return self._name == other._name and set(self._ids) == set(other._ids) + except AttributeError: if other: filename, lineno = frame_codeinfo(currentframe(), 1) _logger.warning("unsupported operand type(s) for \"==\": '%s()' == '%r' (%s:%s)", self._name, other, filename, lineno) - return NotImplemented - return self._name == other._name and set(self._ids) == set(other._ids) + return NotImplemented def __lt__(self, other): - if not isinstance(other, BaseModel) or self._name != other._name: - return NotImplemented - return set(self._ids) < set(other._ids) + try: + if self._name == other._name: + return set(self._ids) < set(other._ids) + except AttributeError: + pass + return NotImplemented def __le__(self, other): - if not isinstance(other, BaseModel) or self._name != other._name: - return NotImplemented - # these are much cheaper checks than a proper subset check, so - # optimise for checking if a null or singleton are subsets of a - # recordset - if not self or self in other: - return True - return set(self._ids) <= set(other._ids) + try: + if self._name == other._name: + # these are much cheaper checks than a proper subset check, so + # optimise for checking if a null or singleton are subsets of a + # recordset + if not self or self in other: + return True + return set(self._ids) <= set(other._ids) + except AttributeError: + pass + return NotImplemented def __gt__(self, other): - if not isinstance(other, BaseModel) or self._name != other._name: - return NotImplemented - return set(self._ids) > set(other._ids) + try: + if self._name == other._name: + return set(self._ids) > set(other._ids) + except AttributeError: + pass + return NotImplemented def __ge__(self, other): - if not isinstance(other, BaseModel) or self._name != other._name: - return NotImplemented - if not other or other in self: - return True - return set(self._ids) >= set(other._ids) + try: + if self._name == other._name: + if not other or other in self: + return True + return set(self._ids) >= set(other._ids) + except AttributeError: + pass + return NotImplemented def __int__(self): return self.id or 0