From 0583a5ddd0b0285b110fb393ef0212c4a7ccb5d4 Mon Sep 17 00:00:00 2001 From: std-odoo Date: Tue, 13 Sep 2022 06:56:31 +0000 Subject: [PATCH] [IMP] base: search on properties fields Purpose ======= Allow to search properties fields. For now, the properties fields are meant to be used with the frontend top bar filter on views in a later commit, and so everything as been kept as simple as possible for that particular use case. Technical ========= Relational properties --------------------- We can not search on relational properties using the rec name, or using one of their fields. The following domains do not work ``` [('properties.partner_id', 'ilike', 'alice')] [('properties.partner_ids', 'ilike', 'alice')] [('properties.user_id.name', 'ilike', 'bob')] [('properties.user_ids.name', 'ilike', 'bob')] ``` Instead, we should give the ids ``` [('properties.partner_id', '=', 55)] [('properties.partner_ids', 'in', 55)] ``` IN operator ----------- The "in" operator is used to either search specific tags / many2many values, either doing a "OR like" condition on "non-array" value. It can be used if the left or the right side is not an array ``` [('properties.my_char', 'in', ['a', 'b'])] [('properties.my_tags', 'in', 'a')] ``` But it can't be used if both side of the condition are array, because it will require to check type in SQL (which is technically possible, but add extra complexity, so for now we keep the feature as simple as possible). So the following domain is not supported. ``` [('properties.my_tags', 'in', ['a', 'b'])] ``` Instead we should use the "OR" operator ``` ['|', ('properties.my_tags', 'in', 'a'), ('properties.my_tags', 'in', 'b')] ``` Tags / many2many - "=" operator ------------------------------- Tags and many2many have to be searched using the "in" operator. So the following domain will return unexpected results and raise a warning. ``` [('attributes.mytags', '=', ['a', 'b'])] ``` The reason is that we don't want to dynamically check the type in SQL, and so we need the "in" domain operator to know what to do in SQL. Task-2980121 Part-of: odoo/odoo#101901 --- .../test_new_api/tests/test_properties.py | 284 +++++++++++++++++- odoo/fields.py | 1 + odoo/models.py | 7 + odoo/osv/expression.py | 55 +++- 4 files changed, 344 insertions(+), 3 deletions(-) diff --git a/odoo/addons/test_new_api/tests/test_properties.py b/odoo/addons/test_new_api/tests/test_properties.py index eed97fbf02d..408d1c6b9ad 100644 --- a/odoo/addons/test_new_api/tests/test_properties.py +++ b/odoo/addons/test_new_api/tests/test_properties.py @@ -3,6 +3,7 @@ import json +import unittest from unittest.mock import patch from odoo import Command @@ -996,7 +997,7 @@ class PropertiesCase(TransactionCase): 'discussion': self.discussion_1.id, 'author': self.user.id, 'attributes': [{ - 'name': 'My Tags', + 'name': 'my_tags', 'type': 'many2many', 'comodel': 'test_new_api.multi.tag', 'value': tags.ids, @@ -1335,7 +1336,6 @@ class PropertiesCase(TransactionCase): } self.assertEqual(expected_properties, sql_properties) - @mute_logger('odoo.fields') def test_properties_field_security(self): """Check the access right related to the Properties fields.""" MultiTag = type(self.env['test_new_api.multi.tag']) @@ -1388,3 +1388,283 @@ class PropertiesCase(TransactionCase): value = self.env.cr.fetchone() self.assertTrue(value and value[0]) return value[0] + + +class PropertiesSearchCase(PropertiesCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.messages = cls.message_1 | cls.message_2 | cls.message_3 + cls.env['test_new_api.message'].search([('id', 'not in', cls.messages.ids)]).unlink() + + @mute_logger('odoo.fields') + def test_properties_field_search_boolean(self): + # search on boolean + self.message_1.attributes = [{ + 'name': 'myboolean', + 'type': 'boolean', + 'value': True, + 'definition_changed': True, + }] + self.message_2.attributes = {'myboolean': False} + messages = self.env['test_new_api.message'].search([('attributes.myboolean', '=', True)]) + self.assertEqual(messages, self.message_1) + messages = self.env['test_new_api.message'].search([('attributes.myboolean', '!=', False)]) + self.assertEqual(messages, self.message_1) + messages = self.env['test_new_api.message'].search([('attributes.myboolean', '=', False)]) + # message 2 has a falsy boolean properties + # message 3 doesn't have the properties (key in dict doesn't exist) + self.assertEqual(messages, self.message_2 | self.message_3) + messages = self.env['test_new_api.message'].search([('attributes.myboolean', '!=', True)]) + self.assertEqual(messages, self.message_2 | self.message_3) + + @mute_logger('odoo.fields') + def test_properties_field_search_char(self): + # search on text properties + self.message_1.attributes = [{ + 'name': 'mychar', + 'type': 'char', + 'value': 'Test', + 'definition_changed': True, + }] + self.message_2.attributes = {'mychar': 'TeSt'} + + messages = self.env['test_new_api.message'].search([('attributes.mychar', '=', 'Test')]) + self.assertEqual(messages, self.message_1, "Should be able to search on a properties field") + messages = self.env['test_new_api.message'].search([('attributes.mychar', '=', '"Test"')]) + self.assertFalse(messages) + messages = self.env['test_new_api.message'].search([('attributes.mychar', 'ilike', 'test')]) + self.assertEqual(messages, self.message_1 | self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.mychar', 'not ilike', 'test')]) + self.assertFalse(messages) + messages = self.env['test_new_api.message'].search([('attributes.mychar', 'ilike', '"test"')]) + self.assertFalse(messages) + + for forbidden_char in '! ()"\'.': + searches = ( + f'mychar{forbidden_char}', + f'my{forbidden_char}char', + f'{forbidden_char}mychar', + ) + for search in searches: + with self.assertRaises(ValueError), self.assertQueryCount(0): + self.env['test_new_api.message'].search([(f'attributes.{search}', '=', 'Test')]) + + # search falsy properties + self.message_3.discussion = self.message_2.discussion + self.message_3.attributes = [{'name': 'mychar', 'value': False}] + self.assertEqual(self._get_sql_properties(self.message_3), {'mychar': False}) + messages = self.env['test_new_api.message'].search([('attributes.mychar', '=', False)]) + self.assertEqual(messages, self.message_3) + + # search falsy properties when the key doesn't exist in the dict + # message 2 properties is False, message 3 properties doesn't exist in database + self.message_2.attributes = [{'name': 'mychar', 'value': False}] + self.env.cr.execute( + "UPDATE test_new_api_message SET attributes = '{}' WHERE id = %s", + [self.message_3.id], + ) + messages = self.env['test_new_api.message'].search([('attributes.mychar', '=', False)]) + self.assertEqual(messages, self.message_2 | self.message_3) + + messages = self.env['test_new_api.message'].search([('attributes.mychar', '!=', False)]) + self.assertEqual(messages, self.message_1) + + # message 1 property contain a string but is not falsy so it's not returned + messages = self.env['test_new_api.message'].search([('attributes.mychar', '!=', True)]) + self.assertEqual(messages, self.message_2 | self.message_3) + + messages = self.env['test_new_api.message'].search([('attributes.mychar', '=', True)]) + self.assertEqual(messages, self.message_1) + + # message 3 is now null instead of being an empty dict + self.env.cr.execute( + "UPDATE test_new_api_message SET attributes = NULL WHERE id = %s", + [self.message_3.id], + ) + + messages = self.env['test_new_api.message'].search([('attributes.mychar', '=', False)]) + self.assertEqual(messages, self.message_2 | self.message_3) + + messages = self.env['test_new_api.message'].search([('attributes.mychar', '!=', False)]) + self.assertEqual(messages, self.message_1) + + @mute_logger('odoo.fields') + def test_properties_field_search_float(self): + # search on float + self.message_1.attributes = [{ + 'name': 'myfloat', + 'type': 'float', + 'value': 3.14, + 'definition_changed': True, + }] + self.message_2.attributes = {'myfloat': 5.55} + messages = self.env['test_new_api.message'].search([('attributes.myfloat', '>', 4.4)]) + self.assertEqual(messages, self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.myfloat', '<', 4.4)]) + self.assertEqual(messages, self.message_1) + messages = self.env['test_new_api.message'].search([('attributes.myfloat', '>', 1.1)]) + self.assertEqual(messages, self.message_1 | self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.myfloat', '<=', 1.1)]) + self.assertFalse(messages) + messages = self.env['test_new_api.message'].search([('attributes.myfloat', '=', 3.14)]) + self.assertEqual(messages, self.message_1) + + @mute_logger('odoo.fields') + def test_properties_field_search_integer(self): + # search on integer + self.messages.discussion = self.discussion_1 + self.message_1.attributes = [{ + 'name': 'myint', + 'type': 'integer', + 'value': 33, + 'definition_changed': True, + }] + self.message_2.attributes = {'myint': 111} + self.message_3.attributes = {'myint': -2} + + messages = self.env['test_new_api.message'].search([('attributes.myint', '>', 4)]) + self.assertEqual(messages, self.message_1 | self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.myint', '<', 4)]) + self.assertEqual(messages, self.message_3) + messages = self.env['test_new_api.message'].search([('attributes.myint', '=', 111)]) + self.assertEqual(messages, self.message_2) + # search on the JSONified value (operator "->>") + messages = self.env['test_new_api.message'].search([('attributes.myint', 'ilike', '1')]) + self.assertEqual(messages, self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.myint', 'not ilike', '1')]) + self.assertEqual(messages, self.message_1 | self.message_3) + + @mute_logger('odoo.fields') + def test_properties_field_search_many2many(self): + self.messages.discussion = self.discussion_1 + partners = self.env['res.partner'].create([{'name': 'A'}, {'name': 'B'}, {'name': 'C'}]) + self.message_1.attributes = [{ + 'name': 'mymany2many', + 'type': 'many2many', + 'comodel': 'res.partner', + 'value': partners.ids, + 'definition_changed': True, + }] + self.message_2.attributes = {'mymany2many': [partners[1].id]} + self.message_3.attributes = {'mymany2many': [partners[2].id]} + messages = self.env['test_new_api.message'].search( + [('attributes.mymany2many', 'in', partners[0].id)]) + self.assertEqual(messages, self.message_1) + messages = self.env['test_new_api.message'].search( + [('attributes.mymany2many', 'in', partners[1].id)]) + self.assertEqual(messages, self.message_1 | self.message_2) + messages = self.env['test_new_api.message'].search( + [('attributes.mymany2many', 'in', partners[2].id)]) + self.assertEqual(messages, self.message_1 | self.message_3) + messages = self.env['test_new_api.message'].search( + [('attributes.mymany2many', 'not in', partners[0].id)]) + self.assertEqual(messages, self.message_2 | self.message_3) + + # IN operator (not supported on many2many and return weird results) + messages = self.env['test_new_api.message'].search( + [('attributes.mymany2many', 'in', [partners[0].id, partners[1].id])]) + self.assertEqual(messages, self.message_2) # should be self.message_1 | self.message_2 + + @mute_logger('odoo.fields') + def test_properties_field_search_many2one(self): + # many2one are just like integer + self.messages.discussion = self.discussion_1 + self.message_1.attributes = [{ + 'name': 'mypartner', + 'type': 'integer', + 'value': self.partner.id, + 'definition_changed': True, + }] + self.message_2.attributes = {'mypartner': self.partner_2.id} + self.message_3.attributes = {'mypartner': False} + + messages = self.env['test_new_api.message'].search( + [('attributes.mypartner', 'in', [self.partner.id, self.partner_2.id])]) + self.assertEqual(messages, self.message_1 | self.message_2) + + messages = self.env['test_new_api.message'].search( + [('attributes.mypartner', 'not in', [self.partner.id, self.partner_2.id])]) + self.assertEqual(messages, self.message_3) + + messages = self.env['test_new_api.message'].search( + [('attributes.mypartner', 'ilike', self.partner.display_name)]) + self.assertFalse(messages, "The ilike on relational properties is not supported") + + @mute_logger('odoo.fields') + def test_properties_field_search_tags(self): + self.messages.discussion = self.discussion_1 + self.message_1.attributes = [{ + 'name': 'mytags', + 'type': 'tags', + 'value': ['a', 'b'], + 'tags': [['a', 'A', 1], ['b', 'B', 2], ['aa', 'AA', 3]], + 'definition_changed': True, + }] + self.message_2.attributes = {'mytags': ['b']} + self.message_3.attributes = {'mytags': ['aa']} + + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'in', 'a')]) + self.assertEqual(messages, self.message_1) + # the search is done on the JSONified value (operator "->>") + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'ilike', 'a')]) + self.assertEqual(messages, self.message_1 | self.message_3) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'not ilike', 'a')]) + self.assertEqual(messages, self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'in', 'b')]) + self.assertEqual(messages, self.message_1 | self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'in', 'aa')]) + self.assertEqual(messages, self.message_3) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'not in', 'b')]) + self.assertEqual(messages, self.message_3) + # the search is done on the JSONified value (operator "->>") + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'ilike', '["aa"]')]) + self.assertEqual(messages, self.message_3) + + # IN operator on array + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'in', [])]) + self.assertFalse(messages) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'not in', [])]) + self.assertEqual(messages, self.message_1 | self.message_2 | self.message_3) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'in', ['a', 'b'])]) + self.assertEqual(messages, self.message_1 | self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'in', ['b', 'a'])]) + self.assertEqual(messages, self.message_1 | self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'in', ['aa'])]) + self.assertEqual(messages, self.message_3) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'in', ['aa', 'b'])]) + self.assertEqual(messages, self.message_3 | self.message_2) + messages = self.env['test_new_api.message'].search([('attributes.mytags', 'not in', ['a', 'b'])]) + self.assertEqual(messages, self.message_3) + + @mute_logger('odoo.fields') + def test_properties_field_search_unaccent(self): + if not self.registry.has_unaccent: + # To enable unaccent feature: + # CREATE EXTENSION unaccent; + raise unittest.SkipTest("unaccent not enabled") + + Model = self.env['test_new_api.message'] + (self.message_1 | self.message_2).discussion = self.discussion_1 + # search on text properties + self.message_1.attributes = [{ + 'name': 'mychar', + 'type': 'char', + 'value': 'Hélène', + 'definition_changed': True, + }] + self.message_2.attributes = {'mychar': 'Helene'} + + result = Model.search([('attributes.mychar', 'ilike', 'Helene')]) + self.assertEqual(self.message_1 | self.message_2, result) + + result = Model.search([('attributes.mychar', 'ilike', 'hélène')]) + self.assertEqual(self.message_1 | self.message_2, result) + + result = Model.search([('attributes.mychar', 'not ilike', 'Helene')]) + self.assertNotIn(self.message_1, result) + self.assertNotIn(self.message_2, result) + + result = Model.search([('attributes.mychar', 'not ilike', 'hélène')]) + self.assertNotIn(self.message_1, result) + self.assertNotIn(self.message_2, result) diff --git a/odoo/fields.py b/odoo/fields.py index cdfe7d271f1..7b4fc7f6bc4 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -3196,6 +3196,7 @@ class Properties(Field): column_type = ('jsonb', 'jsonb') copy = False prefetch = False + unaccent = True write_sequence = 10 # because it must be written after the definition field # the field is computed editable by design (see the compute method below) diff --git a/odoo/models.py b/odoo/models.py index 1213b3ab4d3..49ce8712492 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -68,6 +68,7 @@ from .tools.translate import _, _lt _logger = logging.getLogger(__name__) _unlink = logging.getLogger(__name__ + '.unlink') +regex_alphanumeric = re.compile(r'^[a-z0-9_]+$') regex_order = re.compile(r'^(\s*([a-z0-9:_]+|"[a-z0-9:_]+")(\.id)?(\s+(desc|asc))?\s*(,|$))+(?', '>=', '<', '<=', 'in', 'not in', 'like', 'ilike', 'not like', 'not ilike'): + raise ValueError(f"Wrong search operator {operator!r}") + property_name = path[1] + check_property_field_value_name(property_name) + + if (isinstance(right, bool) or right is None) and operator in ('=', '!='): + # check for boolean value but also for key existence + if right: + # inverse the condition + right = False + operator = '!=' if operator == '=' else '=' + + sql_path = f'"{alias}"."{field.name}"' + key_existence_expr = "" + if operator == '=': # property == False + key_existence_expr = ( + f"OR ({sql_path} IS NULL) " + f"OR NOT ({sql_path} ? '{property_name}')" + ) + + expr = f"(({sql_path} -> '{property_name}') {operator} '%s' {key_existence_expr})" + push_result(expr, [right]) + + else: + if 'like' in operator: + right = f'%{pycompat.to_text(right)}%' + unaccent = self._unaccent(field) + else: + unaccent = lambda x: x + + inverse = '' + arrow = '->' # raw value + if operator in ('in', 'not in'): + if operator == 'not in': + inverse = 'NOT' + if isinstance(right, (list, tuple)): + operator = '<@' + else: + operator = '@>' + right = json.dumps(right) + elif isinstance(right, str): + arrow = '->>' # JSONified value + else: + right = json.dumps(right) + + sql_path = f""""{alias}"."{field.name}" {arrow} '{property_name}'""" + expr = f"""({inverse} ({unaccent(sql_path)}) {operator} ({unaccent('%s')}))""" + push_result(expr, [right]) + # ---------------------------------------- # PATH SPOTTED # -> many2one or one2many with _auto_join: