From fffb8efb5ac745c9feaf796687053fc292a031fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Krier?= Date: Sat, 20 Feb 2021 00:48:50 +0100 Subject: [PATCH] Ensure to join with only one translation row If there are multiple translations for the same record, the join will generate result with multiple times the same id on ModelSQL.search. As DISTINCT ON is not always supported, we must also ensure that duplicated translations are all updated. issue10061 review351261002 --- trytond/ir/translation.py | 43 +++++++++++++------------ trytond/model/fields/field.py | 59 ++++++++++++++++++++-------------- trytond/tests/test_modelsql.py | 58 +++++++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 44 deletions(-) diff --git a/trytond/ir/translation.py b/trytond/ir/translation.py index 4f5faa088b..8321dfef96 100644 --- a/trytond/ir/translation.py +++ b/trytond/ir/translation.py @@ -404,32 +404,34 @@ class Translation(ModelSQL, ModelView): return record.model + ',' + field_name with Transaction().set_context(_check_access=False): - translations = {} + name2translations = defaultdict(list) for translation in cls.search([ ('lang', '=', lang), ('type', '=', ttype), ('name', 'in', [get_name(r) for r in records]), ]): - translations[translation.name] = translation + name2translations[translation.name].append(translation) to_save = [] for record, value in zip(records, values): - translation = translations.get(get_name(record)) + translations = name2translations.get(get_name(record)) if lang == 'en': src = value else: src = getattr(record, field_name) - if not translation: + if not translations: if not src and not value: continue translation = cls() translation.name = name translation.lang = lang translation.type = ttype - translation.src = src - translation.value = value - translation.fuzzy = False - to_save.append(translation) + translations.append(translation) + for translation in translations: + translation.src = src + translation.value = value + translation.fuzzy = False + to_save.append(translation) cls.save(to_save) return @@ -437,7 +439,8 @@ class Translation(ModelSQL, ModelView): with Transaction().set_context(language=Config.get_language()): records = Model.browse(ids) - translations = {} + id2translations = defaultdict(list) + other_translations = defaultdict(list) with Transaction().set_context(_check_access=False): for translation in cls.search([ ('lang', '=', lang), @@ -445,9 +448,8 @@ class Translation(ModelSQL, ModelView): ('name', '=', name), ('res_id', 'in', ids), ]): - translations[translation.res_id] = translation + id2translations[translation.res_id].append(translation) - other_translations = {} if (lang == Config.get_language() and Transaction().context.get('fuzzy_translation', True)): for translation in cls.search([ @@ -456,17 +458,16 @@ class Translation(ModelSQL, ModelView): ('name', '=', name), ('res_id', 'in', ids), ]): - other_translations.setdefault(translation.res_id, [] - ).append(translation) + other_translations[translation.res_id].append(translation) to_save = [] for record, value in zip(records, values): - translation = translations.get(record.id) + translations = id2translations[record.id] if lang == Config.get_language(): src = value else: src = getattr(record, field_name) - if not translation: + if not translations: if not src and not value: continue translation = cls() @@ -474,17 +475,19 @@ class Translation(ModelSQL, ModelView): translation.lang = lang translation.type = ttype translation.res_id = record.id + translations.append(translation) else: - other_langs = other_translations.get(record.id) + other_langs = other_translations[record.id] if other_langs: for other_lang in other_langs: other_lang.src = src other_lang.fuzzy = True to_save.append(other_lang) - translation.value = value - translation.src = src - translation.fuzzy = False - to_save.append(translation) + for translation in translations: + translation.value = value + translation.src = src + translation.fuzzy = False + to_save.append(translation) cls.save(to_save) @classmethod diff --git a/trytond/model/fields/field.py b/trytond/model/fields/field.py index 3ba188a52c..44baf4dc71 100644 --- a/trytond/model/fields/field.py +++ b/trytond/model/fields/field.py @@ -3,11 +3,14 @@ import warnings from functools import wraps +import sql from sql import (operators, Column, Literal, Select, CombiningQuery, Null, Query, Expression, Cast) +from sql.aggregate import Min from sql.conditionals import Coalesce, NullIf from sql.operators import Concat +from trytond import backend from trytond.cache import LRUDictTransaction from trytond.const import OPERATORS from trytond.pool import Pool @@ -16,6 +19,8 @@ from trytond.rpc import RPC from trytond.tools.string_ import StringPartitioned, LazyString from trytond.transaction import Transaction +_sql_version = tuple(map(int, sql.__version__.split('.'))) + def domain_validate(value): assert isinstance(value, list), 'domain must be a list' @@ -492,34 +497,40 @@ class FieldTranslate(Field): def _get_translation_join(self, Model, name, translation, model, table, from_, language): if Model.__name__ == 'ir.model': - return from_.join(translation, 'LEFT', - condition=(translation.name == Concat(Concat( - table.model, ','), name)) - & (translation.res_id == -1) - & (translation.lang == language) - & (translation.type == 'model') - & (translation.fuzzy == Literal(False))) + name_ = Concat(Concat(table.model, ','), name) + type_ = 'model' + res_id = -1 elif Model.__name__ == 'ir.model.field': + name_ = Concat(Concat(model.model, ','), table.name) if name == 'field_description': type_ = 'field' else: type_ = 'help' - return from_.join(model, 'LEFT', - condition=model.id == table.model).join( - translation, 'LEFT', - condition=(translation.name == Concat(Concat( - model.model, ','), table.name)) - & (translation.res_id == -1) - & (translation.lang == language) - & (translation.type == type_) - & (translation.fuzzy == Literal(False))) + res_id = -1 else: - return from_.join(translation, 'LEFT', - condition=(translation.res_id == table.id) - & (translation.name == '%s,%s' % (Model.__name__, name)) - & (translation.lang == language) - & (translation.type == 'model') - & (translation.fuzzy == Literal(False))) + name_ = '%s,%s' % (Model.__name__, name) + type_ = 'model' + res_id = table.id + if backend.name == 'postgresql' and _sql_version >= (1, 1, 0): + query = translation.select( + translation.res_id.as_('res_id'), + translation.value.as_('value'), + distinct=True, + distinct_on=[translation.res_id], + order_by=[translation.res_id, translation.id.desc]) + else: + query = translation.select( + translation.res_id.as_('res_id'), + Min(translation.value).as_('value'), + group_by=[translation.res_id]) + query.where = ( + (translation.lang == language) + & (translation.type == type_) + & (translation.name == name_) + & (translation.fuzzy == Literal(False)) + ) + return query, from_.join(query, 'LEFT', + condition=(query.res_id == res_id)) def convert_domain(self, domain, tables, Model): from trytond.ir.lang import get_parent_language @@ -537,7 +548,7 @@ class FieldTranslate(Field): column = None while language: translation = Translation.__table__() - join = self._get_translation_join( + translation, join = self._get_translation_join( Model, name, translation, model, table, join, language) column = Coalesce(NullIf(column, ''), translation.value) language = get_parent_language(language) @@ -573,7 +584,7 @@ class FieldTranslate(Field): if key not in tables: translation = Translation.__table__() model = IrModel.__table__() - join = self._get_translation_join( + translation, join = self._get_translation_join( Model, name, translation, model, table, table, language) if join.left == table: tables[key] = { diff --git a/trytond/tests/test_modelsql.py b/trytond/tests/test_modelsql.py index e8539e134f..c65a10fa1b 100644 --- a/trytond/tests/test_modelsql.py +++ b/trytond/tests/test_modelsql.py @@ -776,6 +776,64 @@ class ModelSQLTranslationTestCase(TranslationTestCase): self.assertEqual(record.name, "Foo") self.assertEqual(other.name, "Baz") + @with_transaction() + def test_search_unique_result(self): + "Test unique result on search" + pool = Pool() + Model = pool.get('test.modelsql.translation') + Translation = pool.get('ir.translation') + + with Transaction().set_context(language=self.default_language): + record, = Model.create([{'name': "Foo"}]) + with Transaction().set_context(language=self.other_language): + Model.write([record], {'name': "Bar"}) + + translation, = Translation.search([ + ('lang', '=', self.other_language), + ('name', '=', 'test.modelsql.translation,name'), + ('type', '=', 'model'), + ('res_id', '=', record.id), + ]) + Translation.copy([translation], default={'value': "Baz"}) + + with Transaction().set_context(language=self.other_language): + self.assertEqual( + Model.search([('name', 'like', 'Ba%')]), + [record]) + self.assertEqual( + Model.search([], order=[('name', 'DESC')]), + [record]) + + @unittest.skipIf(backend.name != 'postgresql', + "Only PostgreSQL support DISTINCT ON") + @with_transaction() + def test_search_last_translation(self): + "Test unique result on search" + pool = Pool() + Model = pool.get('test.modelsql.translation') + Translation = pool.get('ir.translation') + + with Transaction().set_context(language=self.default_language): + record, = Model.create([{'name': "Foo"}]) + with Transaction().set_context(language=self.other_language): + Model.write([record], {'name': "Bar"}) + + translation, = Translation.search([ + ('lang', '=', self.other_language), + ('name', '=', 'test.modelsql.translation,name'), + ('type', '=', 'model'), + ('res_id', '=', record.id), + ]) + Translation.copy([translation], default={'value': "Baz"}) + + with Transaction().set_context(language=self.other_language): + self.assertEqual( + Model.search([('name', '=', 'Baz')]), + [record]) + self.assertEqual( + Model.search([('name', '=', 'Bar')]), + []) + @with_transaction() def test_search_fill_transaction_cache(self): "Test search fill the transaction cache"