From 44dc331ced8d1a9cf31d8403135a61e4d941d51e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Krier?= Date: Mon, 22 Jun 2015 16:57:06 +0200 Subject: [PATCH] Use Constraint instance in ModelSQL._sql_constraints issue4814 review15491002 --- CHANGELOG | 1 + doc/ref/models/models.rst | 46 ++++++++++- trytond/backend/mysql/table.py | 2 +- trytond/backend/postgresql/table.py | 2 +- trytond/ir/attachment.py | 7 +- trytond/ir/lang.py | 6 +- trytond/ir/model.py | 15 ++-- trytond/ir/module.py | 8 +- trytond/ir/rule.py | 9 ++- trytond/ir/sequence.py | 6 +- trytond/ir/translation.py | 6 +- trytond/ir/trigger.py | 8 +- trytond/model/__init__.py | 2 +- trytond/model/modelsql.py | 114 +++++++++++++++++++--------- trytond/res/group.py | 5 +- trytond/res/user.py | 5 +- trytond/server.py | 7 +- trytond/tests/test.py | 17 +++-- trytond/webdav/webdav.py | 5 +- 19 files changed, 196 insertions(+), 75 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 81806cdfc3..5a247f4b25 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,4 @@ +* Use Constraint instance in ModelSQL._sql_constraints * Only return default record if no domain supplied on ModelSingleton search * Rule.domain_get returns a domain and Rule.query_get a SQL query * Add target_search option to Many2One diff --git a/doc/ref/models/models.rst b/doc/ref/models/models.rst index caa32ce796..d72e6c6df6 100644 --- a/doc/ref/models/models.rst +++ b/doc/ref/models/models.rst @@ -400,11 +400,11 @@ Class attributes are: A list of SQL constraints that are added on the table: - [ ('constraint name', 'SQL constraint', 'error message key'), ... ] + [ ('constraint name', constraint, 'error message key'), ... ] - `constraint name` is the name of the SQL constraint in the database - - `SQL constraint` is the actual SQL constraint + - constraint is an instance of :class:`Constraint` - `error message key` is the key of :attr:`_sql_error_messages` @@ -471,6 +471,48 @@ Class methods: }, } +Constraint +========== + +.. class:: Constraint(table) + +It represents a SQL constraint on a table of the database and it follows the +API of the python-sql expression. + +Instance attributes: + +.. attribute:: Constraint.table + + The SQL Table on which the constraint is defined. + +Check +----- + +.. class:: Check(table, expression) + +It represents a check :class:`Constraint` which enforce the validity of the +expression. + +Instance attributes: + +.. attribute:: Check.expression + + The SQL expression to check. + +Unique +------ + +.. class:: Unique(table, \*columns) + +It represents a unique :class:`Constraint` which enforce the uniqeness of the +group of columns with respect to all the rows in the table. + +Instance attributes: + +.. attribute:: Unique.columns + + The tuple of SQL Column instances. + ======== Workflow ======== diff --git a/trytond/backend/mysql/table.py b/trytond/backend/mysql/table.py index 59d68a1342..8fff8408e8 100644 --- a/trytond/backend/mysql/table.py +++ b/trytond/backend/mysql/table.py @@ -329,7 +329,7 @@ class TableHandler(TableHandlerInterface): try: self.cursor.execute('ALTER TABLE `%s` ' 'ADD CONSTRAINT `%s` %s' - % (self.table_name, ident, constraint,)) + % (self.table_name, ident, constraint), constraint.params) except Exception: if exception: raise diff --git a/trytond/backend/postgresql/table.py b/trytond/backend/postgresql/table.py index 3256fd90b2..4d60c26830 100644 --- a/trytond/backend/postgresql/table.py +++ b/trytond/backend/postgresql/table.py @@ -382,7 +382,7 @@ class TableHandler(TableHandlerInterface): try: self.cursor.execute('ALTER TABLE "%s" ' 'ADD CONSTRAINT "%s" %s' - % (self.table_name, ident, constraint)) + % (self.table_name, ident, constraint), constraint.params) except Exception: if exception: raise diff --git a/trytond/ir/attachment.py b/trytond/ir/attachment.py index 51f6097a77..28bdf1d077 100644 --- a/trytond/ir/attachment.py +++ b/trytond/ir/attachment.py @@ -5,7 +5,7 @@ import hashlib from sql.operators import Concat from sql.conditionals import Coalesce -from ..model import ModelView, ModelSQL, fields +from ..model import ModelView, ModelSQL, fields, Unique from ..config import config from .. import backend from ..transaction import Transaction @@ -56,8 +56,11 @@ class Attachment(ModelSQL, ModelView): def __setup__(cls): super(Attachment, cls).__setup__() cls._order.insert(0, ('last_modification', 'DESC')) + + table = cls.__table__() cls._sql_constraints += [ - ('resource_name', 'UNIQUE(resource, name)', + ('resource_name', + Unique(table, table.resource, table.name), 'The names of attachments must be unique by resource!'), ] diff --git a/trytond/ir/lang.py b/trytond/ir/lang.py index 3210fd6a6b..a6f8c24aa6 100644 --- a/trytond/ir/lang.py +++ b/trytond/ir/lang.py @@ -4,7 +4,7 @@ import datetime import warnings from ast import literal_eval -from ..model import ModelView, ModelSQL, fields +from ..model import ModelView, ModelSQL, fields, Check from ..cache import Cache from ..tools import datetime_strftime from ..transaction import Transaction @@ -49,9 +49,11 @@ class Lang(ModelSQL, ModelView): @classmethod def __setup__(cls): super(Lang, cls).__setup__() + + table = cls.__table__() cls._sql_constraints += [ ('check_decimal_point_thousands_sep', - 'CHECK(decimal_point != thousands_sep)', + Check(table, table.decimal_point != table.thousands_sep), 'decimal_point and thousands_sep must be different!'), ] cls._error_messages.update({ diff --git a/trytond/ir/model.py b/trytond/ir/model.py index 0008ad3825..0e4f72ec16 100644 --- a/trytond/ir/model.py +++ b/trytond/ir/model.py @@ -12,7 +12,7 @@ try: except ImportError: import json -from ..model import ModelView, ModelSQL, fields +from ..model import ModelView, ModelSQL, fields, Unique from ..report import Report from ..wizard import Wizard, StateView, StateAction, Button from ..transaction import Transaction @@ -64,8 +64,9 @@ class Model(ModelSQL, ModelView): @classmethod def __setup__(cls): super(Model, cls).__setup__() + table = cls.__table__() cls._sql_constraints += [ - ('model_uniq', 'UNIQUE(model)', + ('model_uniq', Unique(table, table.model), 'The model must be unique!'), ] cls._error_messages.update({ @@ -249,8 +250,9 @@ class ModelField(ModelSQL, ModelView): @classmethod def __setup__(cls): super(ModelField, cls).__setup__() + table = cls.__table__() cls._sql_constraints += [ - ('name_model_uniq', 'UNIQUE(name, model)', + ('name_model_uniq', Unique(table, table.name, table.model), 'The field name in model must be unique!'), ] cls._error_messages.update({ @@ -770,8 +772,9 @@ class ModelButton(ModelSQL, ModelView): @classmethod def __setup__(cls): super(ModelButton, cls).__setup__() + table = cls.__table__() cls._sql_constraints += [ - ('name_model_uniq', 'UNIQUE(name, model)', + ('name_model_uniq', Unique(table, table.name, table.model), 'The button name in model must be unique!'), ] cls._order.insert(0, ('model', 'ASC')) @@ -838,8 +841,10 @@ class ModelData(ModelSQL, ModelView): @classmethod def __setup__(cls): super(ModelData, cls).__setup__() + table = cls.__table__() cls._sql_constraints = [ - ('fs_id_module_model_uniq', 'UNIQUE("fs_id", "module", "model")', + ('fs_id_module_model_uniq', + Unique(table, table.fs_id, table.module, table.model), 'The triple (fs_id, module, model) must be unique!'), ] cls._buttons.update({ diff --git a/trytond/ir/module.py b/trytond/ir/module.py index 95a8215918..a7859e3b34 100644 --- a/trytond/ir/module.py +++ b/trytond/ir/module.py @@ -4,7 +4,7 @@ from functools import wraps from sql.operators import NotIn -from trytond.model import ModelView, ModelSQL, fields +from trytond.model import ModelView, ModelSQL, fields, Unique from trytond.modules import create_graph, get_module_list, get_module_info from trytond.wizard import Wizard, StateView, Button, StateTransition, \ StateAction @@ -55,8 +55,9 @@ class Module(ModelSQL, ModelView): @classmethod def __setup__(cls): super(Module, cls).__setup__() + table = cls.__table__() cls._sql_constraints = [ - ('name_uniq', 'unique (name)', + ('name_uniq', Unique(table, table.name), 'The name of the module must be unique!'), ] cls._order.insert(0, ('name', 'ASC')) @@ -341,8 +342,9 @@ class ModuleDependency(ModelSQL, ModelView): @classmethod def __setup__(cls): super(ModuleDependency, cls).__setup__() + table = cls.__table__() cls._sql_constraints += [ - ('name_module_uniq', 'UNIQUE(name, module)', + ('name_module_uniq', Unique(table, table.name, table.module), 'Dependency must be unique by module!'), ] diff --git a/trytond/ir/rule.py b/trytond/ir/rule.py index 4bb8b2378b..cb61ffcd57 100644 --- a/trytond/ir/rule.py +++ b/trytond/ir/rule.py @@ -1,6 +1,6 @@ # This file is part of Tryton. The COPYRIGHT file at the top level of # this repository contains the full copyright notices and license terms. -from ..model import ModelView, ModelSQL, fields, EvalEnvironment +from ..model import ModelView, ModelSQL, fields, EvalEnvironment, Check from ..transaction import Transaction from ..cache import Cache from ..pool import Pool @@ -40,10 +40,13 @@ class RuleGroup(ModelSQL, ModelView): cls._order.insert(0, ('model', 'ASC')) cls._order.insert(1, ('global_p', 'ASC')) cls._order.insert(2, ('default_p', 'ASC')) + + t = cls.__table__() cls._sql_constraints += [ - ('global_default_exclusive', 'CHECK(NOT(global_p AND default_p))', + ('global_default_exclusive', + Check(t, (t.global_p == False) | (t.default_p == False)), 'Global and Default are mutually exclusive!'), - ] + ] @staticmethod def default_global_p(): diff --git a/trytond/ir/sequence.py b/trytond/ir/sequence.py index 33d0c18bec..f687760e18 100644 --- a/trytond/ir/sequence.py +++ b/trytond/ir/sequence.py @@ -5,7 +5,7 @@ import time from itertools import izip from sql import Flavor -from ..model import ModelView, ModelSQL, fields +from ..model import ModelView, ModelSQL, fields, Check from ..tools import datetime_strftime from ..pyson import Eval, And from ..transaction import Transaction @@ -84,8 +84,10 @@ class Sequence(ModelSQL, ModelView): @classmethod def __setup__(cls): super(Sequence, cls).__setup__() + table = cls.__table__() cls._sql_constraints += [ - ('check_timestamp_rounding', 'CHECK(timestamp_rounding > 0)', + ('check_timestamp_rounding', + Check(table, table.timestamp_rounding > 0), 'Timestamp rounding should be greater than 0'), ] cls._error_messages.update({ diff --git a/trytond/ir/translation.py b/trytond/ir/translation.py index a48c7815fb..2d3b6abb50 100644 --- a/trytond/ir/translation.py +++ b/trytond/ir/translation.py @@ -19,7 +19,7 @@ from sql.conditionals import Case from sql.operators import Or, And from sql.aggregate import Max -from ..model import ModelView, ModelSQL, fields +from ..model import ModelView, ModelSQL, fields, Unique from ..wizard import Wizard, StateView, StateTransition, StateAction, \ Button from ..tools import file_open, reduce_ids, grouped_slice @@ -80,9 +80,11 @@ class Translation(ModelSQL, ModelView): @classmethod def __setup__(cls): super(Translation, cls).__setup__() + t = cls.__table__() cls._sql_constraints += [ ('translation_md5_uniq', - 'UNIQUE (name, res_id, lang, type, src_md5, module)', + Unique(t, + t.name, t.res_id, t.lang, t.type, t.src_md5, t.module), 'Translation must be unique'), ] cls._error_messages.update({ diff --git a/trytond/ir/trigger.py b/trytond/ir/trigger.py index 23b8a79a14..bef332032a 100644 --- a/trytond/ir/trigger.py +++ b/trytond/ir/trigger.py @@ -5,7 +5,7 @@ import time from sql import Literal, Null from sql.aggregate import Count, Max -from ..model import ModelView, ModelSQL, fields, EvalEnvironment +from ..model import ModelView, ModelSQL, fields, EvalEnvironment, Check from ..pyson import Eval, PYSONDecoder from ..tools import grouped_slice from .. import backend @@ -56,9 +56,13 @@ class Trigger(ModelSQL, ModelView): @classmethod def __setup__(cls): super(Trigger, cls).__setup__() + t = cls.__table__() cls._sql_constraints += [ ('on_exclusive', - 'CHECK(NOT(on_time AND (on_create OR on_write OR on_delete)))', + Check(t, ~((t.on_time == True) + & ((t.on_create == True) + | (t.on_write == True) + | (t.on_delete == True)))), '"On Time" and others are mutually exclusive!'), ] cls._error_messages.update({ diff --git a/trytond/model/__init__.py b/trytond/model/__init__.py index 1a71ef8da0..2d845c6aa3 100644 --- a/trytond/model/__init__.py +++ b/trytond/model/__init__.py @@ -4,7 +4,7 @@ from .model import Model from .modelview import ModelView from .modelstorage import ModelStorage, EvalEnvironment from .modelsingleton import ModelSingleton -from .modelsql import ModelSQL +from .modelsql import ModelSQL, Check, Unique from .workflow import Workflow from .dictschema import DictSchemaMixin from .match import MatchMixin diff --git a/trytond/model/modelsql.py b/trytond/model/modelsql.py index 0bd8c639f0..ca5c524c1d 100644 --- a/trytond/model/modelsql.py +++ b/trytond/model/modelsql.py @@ -1,12 +1,10 @@ # 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 re import datetime -from functools import reduce from itertools import islice, izip, chain, ifilter from collections import OrderedDict -from sql import Table, Column, Literal, Desc, Asc, Expression, Flavor, Null +from sql import Table, Column, Literal, Desc, Asc, Expression, Null from sql.functions import Now, Extract from sql.conditionals import Coalesce from sql.operators import Or, And, Operator @@ -25,8 +23,67 @@ from trytond.rpc import RPC from .modelstorage import cache_size -_RE_UNIQUE = re.compile('UNIQUE\s*\((.*)\)', re.I) -_RE_CHECK = re.compile('CHECK\s*\((.*)\)', re.I) + +class Constraint(object): + __slots__ = ('_table',) + + def __init__(self, table): + assert isinstance(table, Table) + self._table = table + + @property + def table(self): + return self._table + + def __str__(self): + raise NotImplementedError + + @property + def params(self): + raise NotImplementedError + + +class Check(Constraint): + __slots__ = ('_expression',) + + def __init__(self, table, expression): + super(Check, self).__init__(table) + assert isinstance(expression, Expression) + self._expression = expression + + @property + def expression(self): + return self._expression + + def __str__(self): + return 'CHECK(%s)' % self.expression + + @property + def params(self): + return self.expression.params + + +class Unique(Constraint): + __slots__ = ('_columns',) + + def __init__(self, table, *columns): + super(Unique, self).__init__(table) + assert all(isinstance(col, Column) for col in columns) + self._columns = tuple(columns) + + @property + def columns(self): + return self._columns + + def __str__(self): + return 'UNIQUE(%s)' % (', '.join(map(str, self.columns))) + + @property + def params(self): + p = [] + for column in self.columns: + p.extend(column.params) + return tuple(p) class ModelSQL(ModelStorage): @@ -1310,47 +1367,36 @@ class ModelSQL(ModelStorage): return # Works only for a single transaction ids = map(int, records) - table = cls.__table__() - param = Flavor.get().param for _, sql, error in cls._sql_constraints: - match = _RE_UNIQUE.match(sql) - if match: - sql = match.group(1) - columns = sql.split(',') - sql_clause = ' AND '.join('%s = %s' - % (i, param) for i in columns) - sql_clause = '(id != ' + param + ' AND ' + sql_clause + ')' - + table = sql.table + if isinstance(sql, Unique): + columns = [Column(table, c.name) for c in sql.columns] + columns.insert(0, table.id) in_max = cursor.IN_MAX / (len(columns) + 1) for sub_ids in grouped_slice(ids, in_max): red_sql = reduce_ids(table.id, sub_ids) - cursor.execute('SELECT id,' + sql + ' ' - 'FROM "' + cls._table + '" ' - 'WHERE ' + str(red_sql), red_sql.params) - - fetchall = cursor.fetchall() - cursor.execute('SELECT id ' - 'FROM "' + cls._table + '" ' - 'WHERE ' + - ' OR '.join((sql_clause,) * len(fetchall)), - reduce(lambda x, y: x + list(y), fetchall, [])) + cursor.execute(*table.select(*columns, where=red_sql)) + where = Literal(False) + for row in cursor.fetchall(): + clause = table.id != row[0] + for column, value in zip(sql.columns, row[1:]): + if value is None: + # NULL is always unique + clause &= Literal(False) + clause &= Column(table, column.name) == value + where |= clause + cursor.execute(*table.select(table.id, where=where)) if cursor.fetchone(): cls.raise_user_error(error) - continue - match = _RE_CHECK.match(sql) - if match: - sql = match.group(1) + elif isinstance(sql, Check): for sub_ids in grouped_slice(ids): red_sql = reduce_ids(table.id, sub_ids) - cursor.execute('SELECT id ' - 'FROM "' + cls._table + '" ' - 'WHERE NOT (' + sql + ') ' - 'AND ' + str(red_sql), red_sql.params) + cursor.execute(*table.select(table.id, + where=~sql.expression & red_sql)) if cursor.fetchone(): cls.raise_user_error(error) - continue def convert_from(table, tables): diff --git a/trytond/res/group.py b/trytond/res/group.py index 03772c8c47..528e5fcc27 100644 --- a/trytond/res/group.py +++ b/trytond/res/group.py @@ -2,7 +2,7 @@ # this repository contains the full copyright notices and license terms. "Group" from itertools import chain -from ..model import ModelView, ModelSQL, fields +from ..model import ModelView, ModelSQL, fields, Unique from ..pool import Pool, PoolMeta from ..tools import grouped_slice @@ -46,8 +46,9 @@ class Group(ModelSQL, ModelView): @classmethod def __setup__(cls): super(Group, cls).__setup__() + table = cls.__table__() cls._sql_constraints += [ - ('name_uniq', 'unique (name)', + ('name_uniq', Unique(table, table.name), 'The name of the group must be unique!') ] diff --git a/trytond/res/user.py b/trytond/res/user.py index f99fc5bfff..b9d05a27e2 100644 --- a/trytond/res/user.py +++ b/trytond/res/user.py @@ -21,7 +21,7 @@ try: except ImportError: bcrypt = None -from ..model import ModelView, ModelSQL, fields +from ..model import ModelView, ModelSQL, fields, Unique from ..wizard import Wizard, StateView, Button, StateTransition from ..tools import grouped_slice from .. import backend @@ -81,8 +81,9 @@ class User(ModelSQL, ModelView): 'set_preferences': RPC(readonly=False, check_access=False), 'get_preferences_fields_view': RPC(check_access=False), }) + table = cls.__table__() cls._sql_constraints += [ - ('login_key', 'UNIQUE (login)', + ('login_key', Unique(table, table.login), 'You can not have two users with the same login!') ] cls._preferences_fields = [ diff --git a/trytond/server.py b/trytond/server.py index 4d420ce827..e75f6f3743 100644 --- a/trytond/server.py +++ b/trytond/server.py @@ -13,6 +13,8 @@ import time from getpass import getpass import threading +from sql import Table + from trytond.config import config, parse_listen from trytond import backend from trytond.pool import Pool @@ -101,8 +103,9 @@ class TrytonServer(object): if not cursor.test(): raise Exception("'%s' is not a Tryton database!" % db_name) - cursor.execute('SELECT code FROM ir_lang ' - 'WHERE translatable = %s', (True,)) + lang = Table('ir_lang') + cursor.execute(*lang.select(lang.code, + where=lang.translatable == True)) lang = [x[0] for x in cursor.fetchall()] else: lang = None diff --git a/trytond/tests/test.py b/trytond/tests/test.py index d6377dd56a..b493a279f9 100644 --- a/trytond/tests/test.py +++ b/trytond/tests/test.py @@ -3,7 +3,7 @@ import datetime from decimal import Decimal -from trytond.model import ModelSQL, DictSchemaMixin, fields +from trytond.model import ModelSQL, DictSchemaMixin, fields, Unique from trytond.pyson import Eval __all__ = [ @@ -365,10 +365,11 @@ class One2OneRelation(ModelSQL): @classmethod def __setup__(cls): super(One2OneRelation, cls).__setup__() + table = cls.__table__() cls._sql_constraints += [ - ('origin_unique', 'UNIQUE(origin)', + ('origin_unique', Unique(table, table.origin), 'Origin must be unique'), - ('target_unique', 'UNIQUE(target)', + ('target_unique', Unique(table, table.target), 'Target must be unique'), ] @@ -390,10 +391,11 @@ class One2OneRequiredRelation(ModelSQL): @classmethod def __setup__(cls): super(One2OneRequiredRelation, cls).__setup__() + table = cls.__table__() cls._sql_constraints += [ - ('origin_unique', 'UNIQUE(origin)', + ('origin_unique', Unique(table, table.origin), 'Origin must be unique'), - ('target_unique', 'UNIQUE(target)', + ('target_unique', Unique(table, table.target), 'Target must be unique'), ] @@ -416,10 +418,11 @@ class One2OneDomainRelation(ModelSQL): @classmethod def __setup__(cls): super(One2OneDomainRelation, cls).__setup__() + table = cls.__table__() cls._sql_constraints += [ - ('origin_unique', 'UNIQUE(origin)', + ('origin_unique', Unique(table, table.origin), 'Origin must be unique'), - ('target_unique', 'UNIQUE(target)', + ('target_unique', Unique(table, table.target), 'Target must be unique'), ] diff --git a/trytond/webdav/webdav.py b/trytond/webdav/webdav.py index f34561c2db..69d546f6be 100644 --- a/trytond/webdav/webdav.py +++ b/trytond/webdav/webdav.py @@ -14,7 +14,7 @@ from dateutil.relativedelta import relativedelta from sql.functions import Extract from sql.conditionals import Coalesce -from trytond.model import ModelView, ModelSQL, fields +from trytond.model import ModelView, ModelSQL, fields, Unique from trytond.tools import reduce_ids from trytond.transaction import Transaction from trytond.pool import Pool @@ -57,8 +57,9 @@ class Collection(ModelSQL, ModelView): @classmethod def __setup__(cls): super(Collection, cls).__setup__() + table = cls.__table__() cls._sql_constraints += [ - ('name_parent_uniq', 'UNIQUE (name, parent)', + ('name_parent_uniq', Unique(table, table.name, table.parent), 'The collection name must be unique inside a collection!'), ] cls._error_messages.update({