From d1bb790cb87082b9831845fdcb4b9c871e25caa2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Krier?= Date: Fri, 21 Dec 2018 23:41:56 +0100 Subject: [PATCH] New API to read related fields issue7888 review58431002 --- CHANGELOG | 1 + doc/ref/models/models.rst | 3 + trytond/model/model.py | 10 +- trytond/model/modelsingleton.py | 6 +- trytond/model/modelsql.py | 146 +++++++++--------- trytond/tests/modelsql.py | 20 +++ trytond/tests/test_access.py | 13 +- trytond/tests/test_modelsingleton.py | 2 +- trytond/tests/test_modelsql.py | 215 +++++++++++++++++++++++++++ 9 files changed, 331 insertions(+), 85 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 0c8b3a5bda..bca327db8e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,4 @@ +* New API to read related fields * Remove implicit fields names in ModelStorage.read * Check read access on field in search domain (issue7766) * Add active field on ModelAccess, ModelFieldAccess and Group diff --git a/doc/ref/models/models.rst b/doc/ref/models/models.rst index 847732761c..15fb6f2516 100644 --- a/doc/ref/models/models.rst +++ b/doc/ref/models/models.rst @@ -256,6 +256,9 @@ Class methods: Return a list of dictionary for the record ids. The dictionary is composed of the fields as key and their values. + ``fields_names`` can contain dereferenced fields from related models. + Their values will be returned under the referencing field suffixed by a + `.`. The number of *dots* in the name is not limited. The order of the returned list is not guaranteed. .. classmethod:: ModelStorage.write(records, values, [[records, values], ...]) diff --git a/trytond/model/model.py b/trytond/model/model.py index 9693e1352e..913be98e3f 100644 --- a/trytond/model/model.py +++ b/trytond/model/model.py @@ -151,13 +151,9 @@ class Model(WarningErrorMixin, URLMixin, PoolBase, metaclass=ModelMeta): and value.get(field_name)): Target = pool.get(field.model_name) if 'rec_name' in Target._fields: - value[field_name + '.rec_name'] = Target( - value[field_name]).rec_name - - if not with_rec_name: - for field in list(value.keys()): - if field.endswith('.rec_name'): - del value[field] + value.setdefault( + field_name + '.', {})['rec_name'] = Target( + value[field_name]).rec_name return value @classmethod diff --git a/trytond/model/modelsingleton.py b/trytond/model/modelsingleton.py index e50ef63417..752ff9820a 100644 --- a/trytond/model/modelsingleton.py +++ b/trytond/model/modelsingleton.py @@ -37,9 +37,6 @@ class ModelSingleton(ModelStorage): for field_name in fields_names: if field_name not in res: res[field_name] = None - for field_name in list(res.keys()): - if field_name not in fields_names: - del res[field_name] res['id'] = ids[0] return [res] res = super(ModelSingleton, cls).read([singleton.id], fields_names) @@ -107,6 +104,5 @@ class ModelSingleton(ModelStorage): if cls._fields[field]._type in ('many2one',): fields_names.append(field + '.rec_name') default, = cls.read([singleton.id], fields_names=fields_names) - for field in (x for x in list(default.keys()) if x not in fields_names): - del default[field] + del default['id'] return default diff --git a/trytond/model/modelsql.py b/trytond/model/modelsql.py index bb5cd640e7..0b7c3fbfd1 100644 --- a/trytond/model/modelsql.py +++ b/trytond/model/modelsql.py @@ -1,8 +1,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 datetime -from itertools import islice, chain -from collections import OrderedDict +from itertools import islice, chain, product +from collections import OrderedDict, defaultdict from functools import wraps from sql import (Table, Column, Literal, Desc, Asc, Expression, Null, @@ -824,82 +824,90 @@ class ModelSQL(ModelStorage): for row in result: row[fname] = getter_result[row['id']] + def group_by(rows, keyfunc=None): + result = defaultdict(list) + if keyfunc is None: + result[None] = rows + else: + for row in rows: + result[keyfunc(row)].append(row) + return result + + def read_related(field, Target, rows, fields): + name = field.name + target_ids = [] + if field._type.endswith('2many'): + add = target_ids.extend + elif field._type == 'reference': + def add(value): + id_ = int(value.split(',', 1)[1]) + if id_ >= 0: + target_ids.append(id_) + else: + add = target_ids.append + for row in rows: + value = row[name] + if value is not None: + add(value) + return Target.read(target_ids, fields) + + def add_related(field, rows, targets): + name = field.name + key = name + '.' + if field._type.endswith('2many'): + for row in rows: + row[key] = values = list() + for target in row[name]: + if target is not None: + values.append(targets[target]) + else: + for row in rows: + value = row[name] + if isinstance(value, str): + value = int(value.split(',', 1)[1]) + if value is not None and value >= 0: + row[key] = targets[value] + else: + row[key] = None + to_del = set() - fields_related2values = {} - for fname in list(fields_related.keys()) + datetime_fields: + for fname in chain(fields_related.keys(), datetime_fields): if fname not in fields_names: to_del.add(fname) if fname not in cls._fields: continue if fname not in fields_related: continue - fields_related2values.setdefault(fname, {}) field = cls._fields[fname] - if field._type in ('many2one', 'one2one'): - if hasattr(field, 'model_name'): - Target = pool.get(field.model_name) - else: - Target = field.get_target() - if getattr(field, 'datetime_field', None): - for row in result: - if row[fname] is None: - continue - with Transaction().set_context( - _datetime=row[field.datetime_field]): - date_target, = Target.read([row[fname]], - fields_related[fname]) - target_id = date_target.pop('id') - fields_related2values[fname].setdefault(target_id, {}) - fields_related2values[ - fname][target_id][row['id']] = date_target - else: - for target in Target.read( - [r[fname] for r in result if r[fname]], - fields_related[fname]): - target_id = target.pop('id') - fields_related2values[fname].setdefault(target_id, {}) - for row in result: - fields_related2values[ - fname][target_id][row['id']] = target - elif field._type == 'reference': - for row in result: - if not row[fname]: - continue - model_name, record_id = row[fname].split(',', 1) - if not model_name: - continue - record_id = int(record_id) - if record_id < 0: - continue - Target = pool.get(model_name) - target, = Target.read([record_id], fields_related[fname]) - del target['id'] - fields_related2values[fname][row[fname]] = target + datetime_field = getattr(field, 'datetime_field', None) + if field._type == 'reference': + def keyfunc(row): + value = row[fname] + if not value: + return (None, None) + model, _ = value.split(',', 1) + return (pool.get(model), row.get(datetime_field)) + else: + Target = field.get_target() - if to_del or fields_related or datetime_fields: - for row in result: - for fname in fields_related: - if fname not in cls._fields: - continue - field = cls._fields[fname] - for related in fields_related[fname]: - related_name = '%s.%s' % (fname, related) - value = None - if row[fname]: - if field._type in ('many2one', 'one2one'): - value = fields_related2values[fname][ - row[fname]][row['id']][related] - elif field._type == 'reference': - model_name, record_id = row[fname - ].split(',', 1) - if model_name: - record_id = int(record_id) - if record_id >= 0: - value = fields_related2values[fname][ - row[fname]][related] - row[related_name] = value - for field in to_del: - del row[field] + def keyfunc(row): + return (Target, row.get(datetime_field)) + for (Target, datetime_), rows in group_by( + result, keyfunc).items(): + ctx = {} + if datetime_: + ctx['_datetime'] = datetime_ + with Transaction().set_context(ctx): + if Target: + targets = read_related( + field, Target, rows, fields_related[fname]) + targets = {t['id']: t for t in targets} + else: + targets = {} + add_related(field, rows, targets) + + for row, field in product(result, to_del): + del row[field] return result diff --git a/trytond/tests/modelsql.py b/trytond/tests/modelsql.py index 1dcbae435b..18bad3d3b3 100644 --- a/trytond/tests/modelsql.py +++ b/trytond/tests/modelsql.py @@ -6,6 +6,24 @@ from trytond.model import ModelSQL, fields, Check, Unique, Exclude from trytond.pool import Pool +class ModelSQLRead(ModelSQL): + "ModelSQL to test read" + __name__ = 'test.modelsql.read' + name = fields.Char("Name") + target = fields.Many2One('test.modelsql.read.target', "Target") + targets = fields.One2Many('test.modelsql.read.target', 'parent', "Targets") + reference = fields.Reference( + "Reference", [(None, ""), ('test.modelsql.read.target', "Target")]) + + +class ModelSQLReadTarget(ModelSQL): + "ModelSQL Target to test read" + __name__ = 'test.modelsql.read.target' + name = fields.Char("Name") + parent = fields.Many2One('test.modelsql.read', "Parent") + target = fields.Many2One('test.modelsql.read.target', "Target") + + class ModelSQLRequiredField(ModelSQL): 'model with a required field' __name__ = 'test.modelsql' @@ -106,6 +124,8 @@ class ModelExclude(ModelSQL): def register(module): Pool.register( + ModelSQLRead, + ModelSQLReadTarget, ModelSQLRequiredField, ModelSQLTimestamp, ModelSQLFieldSet, diff --git a/trytond/tests/test_access.py b/trytond/tests/test_access.py index 0a3f183ecc..f76747084b 100644 --- a/trytond/tests/test_access.py +++ b/trytond/tests/test_access.py @@ -271,43 +271,50 @@ class ModelAccessReadTestCase(_ModelAccessTestCase): TestAccess.search([]) @with_transaction(context=_context) - def test_access_search_relate_empty(self): + def test_access_relate_empty(self): "Test access on search relate without model access" pool = Pool() TestAccess = pool.get('test.access') + record, = TestAccess.create([{}]) + TestAccess.read([record.id], ['relate.value']) TestAccess.search([('relate.value', '=', 42)]) TestAccess.search([('reference.value', '=', 42, 'test.access.relate')]) @with_transaction(context=_context) - def test_access_search_relate(self): + def test_access_relate(self): "Test access on search relate" pool = Pool() TestAccess = pool.get('test.access') Model = pool.get('ir.model') ModelAccess = pool.get('ir.model.access') + record, = TestAccess.create([{}]) model, = Model.search([('model', '=', 'test.access.relate')]) ModelAccess.create([{ 'model': model.id, 'perm_read': True, }]) + TestAccess.read([record.id], ['relate.value']) TestAccess.search([('relate.value', '=', 42)]) TestAccess.search([('reference.value', '=', 42, 'test.access.relate')]) @with_transaction(context=_context) - def test_no_access_search_relate(self): + def test_no_access_relate(self): "Test no access on search relate" pool = Pool() TestAccess = pool.get('test.access') Model = pool.get('ir.model') ModelAccess = pool.get('ir.model.access') + record, = TestAccess.create([{}]) model, = Model.search([('model', '=', 'test.access.relate')]) ModelAccess.create([{ 'model': model.id, 'perm_read': False, }]) + with self.assertRaises(UserError): + TestAccess.read([record.id], ['relate.value']) with self.assertRaises(UserError): TestAccess.search([('relate.value', '=', 42)]) with self.assertRaises(UserError): diff --git a/trytond/tests/test_modelsingleton.py b/trytond/tests/test_modelsingleton.py index 576e6bfc14..596ce485a9 100644 --- a/trytond/tests/test_modelsingleton.py +++ b/trytond/tests/test_modelsingleton.py @@ -37,7 +37,7 @@ class ModelSingletonTestCase(unittest.TestCase): 'write_date', ]) self.assertEqual(singleton['create_uid'], Transaction().user) - self.assertEqual(singleton['create_uid.rec_name'], 'Administrator') + self.assertEqual(singleton['create_uid.']['rec_name'], 'Administrator') self.assertTrue(isinstance(singleton['create_date'], datetime)) self.assertEqual(singleton['write_uid'], None) self.assertEqual(singleton['write_date'], None) diff --git a/trytond/tests/test_modelsql.py b/trytond/tests/test_modelsql.py index 179e9a8174..c940efb148 100644 --- a/trytond/tests/test_modelsql.py +++ b/trytond/tests/test_modelsql.py @@ -20,6 +20,221 @@ class ModelSQLTestCase(unittest.TestCase): def setUpClass(cls): activate_module('tests') + @with_transaction() + def test_read(self): + "Test simple read" + pool = Pool() + Model = pool.get('test.modelsql.read') + + foo, bar = Model.create([{'name': "Foo"}, {'name': "Bar"}]) + values = Model.read([foo.id, bar.id], ['name']) + + self.assertEqual( + sorted(values, key=lambda v: v['id']), + [{'id': foo.id, 'name': "Foo"}, {'id': bar.id, 'name': "Bar"}]) + + @with_transaction() + def test_read_related_2one(self): + "Test read with related Many2One" + pool = Pool() + Model = pool.get('test.modelsql.read') + Target = pool.get('test.modelsql.read.target') + + target, = Target.create([{'name': "Target"}]) + record, = Model.create([{'target': target.id}]) + values = Model.read([record.id], ['target.name']) + + self.assertEqual(values, [{ + 'id': record.id, + 'target.': { + 'id': target.id, + 'name': "Target", + }, + }]) + + @with_transaction() + def test_read_related_2one_empty(self): + "Test read with empty related Many2One" + pool = Pool() + Model = pool.get('test.modelsql.read') + + record, = Model.create([{'target': None}]) + values = Model.read([record.id], ['target.name']) + + self.assertEqual(values, [{ + 'id': record.id, + 'target.': None, + }]) + + @with_transaction() + def test_read_related_reference(self): + "Test read with related Reference" + pool = Pool() + Model = pool.get('test.modelsql.read') + Target = pool.get('test.modelsql.read.target') + + target, = Target.create([{'name': "Target"}]) + record, = Model.create([{'reference': str(target)}]) + values = Model.read([record.id], ['reference.name']) + + self.assertEqual(values, [{ + 'id': record.id, + 'reference.': { + 'id': target.id, + 'name': "Target", + }, + }]) + + @with_transaction() + def test_read_related_reference_empty(self): + "Test read with empty related Reference" + pool = Pool() + Model = pool.get('test.modelsql.read') + + record, = Model.create([{'name': "Foo", 'reference': None}]) + values = Model.read([record.id], ['reference.name']) + + self.assertEqual(values, [{ + 'id': record.id, + 'reference.': None, + }]) + + @with_transaction() + def test_read_related_2many(self): + "Test read with related One2Many" + pool = Pool() + Model = pool.get('test.modelsql.read') + Target = pool.get('test.modelsql.read.target') + + target, = Target.create([{'name': "Target"}]) + record, = Model.create( + [{'targets': [('add', [target.id])]}]) + values = Model.read([record.id], ['targets.name']) + + self.assertEqual(values, [{ + 'id': record.id, + 'targets.': [{ + 'id': target.id, + 'name': "Target", + }], + }]) + + @with_transaction() + def test_read_related_2many_empty(self): + "Test read with empty related One2Many" + pool = Pool() + Model = pool.get('test.modelsql.read') + + record, = Model.create( + [{'targets': None}]) + values = Model.read([record.id], ['targets.name']) + + self.assertEqual(values, [{ + 'id': record.id, + 'targets.': [], + }]) + + @with_transaction() + def test_read_related_2many_multiple(self): + "Test read with multiple related One2Many" + pool = Pool() + Model = pool.get('test.modelsql.read') + Target = pool.get('test.modelsql.read.target') + + target1, target2 = Target.create([ + {'name': "Target 1"}, + {'name': "Target 2"}]) + record, = Model.create( + [{'targets': [('add', [target1.id, target2.id])]}]) + values = Model.read([record.id], ['targets.name']) + + self.assertEqual(values, [{ + 'id': record.id, + 'targets.': [{ + 'id': target1.id, + 'name': "Target 1", + }, { + 'id': target2.id, + 'name': "Target 2", + }], + }]) + + @with_transaction() + def test_read_related_mixed(self): + "Test read mixed related" + pool = Pool() + Model = pool.get('test.modelsql.read') + Target = pool.get('test.modelsql.read.target') + + target1, target2, target3 = Target.create([ + {'name': "Target 1"}, + {'name': "Target 2"}, + {'name': "Target 3"}]) + record1, record2 = Model.create([{ + 'name': "Foo", + 'target': target1.id, + 'targets': [('add', [target1.id, target2.id])], + }, { + 'name': "Bar", + 'reference': str(target2), + 'targets': [('add', [target3.id])], + }]) + values = Model.read( + [record1.id, record2.id], + ['name', 'target', 'target.name', 'targets', 'targets.name']) + + self.assertEqual( + sorted(values, key=lambda v: v['id']), [{ + 'id': record1.id, + 'name': "Foo", + 'target': target1.id, + 'target.': { + 'id': target1.id, + 'name': "Target 1", + }, + 'targets': (target1.id, target2.id), + 'targets.': [{ + 'id': target1.id, + 'name': "Target 1", + }, { + 'id': target2.id, + 'name': "Target 2", + }], + }, { + 'id': record2.id, + 'name': "Bar", + 'target': None, + 'target.': None, + 'targets': (target3.id,), + 'targets.': [{ + 'id': target3.id, + 'name': "Target 3", + }], + }]) + + @with_transaction() + def test_read_related_nested(self): + "Test read with nested related" + pool = Pool() + Model = pool.get('test.modelsql.read') + Target = pool.get('test.modelsql.read.target') + + target, = Target.create([{'name': "Target"}]) + record, = Model.create( + [{'name': "Record", 'targets': [('add', [target.id])]}]) + values = Model.read([record.id], ['targets.parent.name']) + + self.assertEqual(values, [{ + 'id': record.id, + 'targets.': [{ + 'id': target.id, + 'parent.': { + 'id': record.id, + 'name': "Record", + }, + }], + }]) + @unittest.skipIf(backend.name() == 'sqlite', 'SQLite not concerned because tryton don\'t set "NOT NULL"' 'constraint: "ALTER TABLE" don\'t support NOT NULL constraint'