From fbdf1817de46923b9fc3acc0b56bb527428d27c0 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Thu, 24 Mar 2011 12:10:55 +0100 Subject: [PATCH 01/17] [FIX] yaml_tag: python records should log AssertionErrors at ERROR level by default bzr revid: odo@openerp.com-20110324111055-nlnv42jrgtdihens --- openerp/tools/yaml_tag.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openerp/tools/yaml_tag.py b/openerp/tools/yaml_tag.py index 20aabf1c54c..ba61b4825ca 100644 --- a/openerp/tools/yaml_tag.py +++ b/openerp/tools/yaml_tag.py @@ -32,7 +32,7 @@ class Record(YamlTag): return '!record {model: %s, id: %s}:' % (str(self.model,), str(self.id,)) class Python(YamlTag): - def __init__(self, model, severity=logging.WARNING, name="", **kwargs): + def __init__(self, model, severity=logging.ERROR, name="", **kwargs): self.model= model self.severity = severity self.name = name From 6a46a151858622a20c6c129405a222ed88dc5a86 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Thu, 24 Mar 2011 12:14:27 +0100 Subject: [PATCH 02/17] [IMP] ir.model.access: split method for generating the list of groups that have a given access right This is useful when modules later need to override it, for example when adding hidden groups that should not be mentioned there (this is what the share module does). bzr revid: odo@openerp.com-20110324111427-puz4jtrem9ceibxf --- openerp/addons/base/ir/ir_model.py | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/openerp/addons/base/ir/ir_model.py b/openerp/addons/base/ir/ir_model.py index 5ce50370c77..6c11a7adf56 100644 --- a/openerp/addons/base/ir/ir_model.py +++ b/openerp/addons/base/ir/ir_model.py @@ -451,6 +451,23 @@ class ir_model_access(osv.osv): # pass no groups -> no access return False + def group_names_with_access(self, cr, model_name, access_mode): + """Returns the names of visible groups which have been granted ``access_mode`` on + the model ``model_name``. + :rtype: list + """ + assert access_mode in ['read','write','create','unlink'], 'Invalid access mode: %s' % access_mode + cr.execute('''SELECT + g.name + FROM + ir_model_access a + JOIN ir_model m ON (a.model_id=m.id) + JOIN res_groups g ON (a.group_id=g.id) + WHERE + m.model=%s AND + a.perm_''' + access_mode, (model_name,)) + return [x[0] for x in cr.fetchall()] + def check(self, cr, uid, model, mode='read', raise_exception=True, context=None): if uid==1: # User root have all accesses @@ -494,16 +511,7 @@ class ir_model_access(osv.osv): r = cr.fetchone()[0] if not r and raise_exception: - cr.execute('''select - g.name - from - ir_model_access a - left join ir_model m on (a.model_id=m.id) - left join res_groups g on (a.group_id=g.id) - where - m.model=%s and - a.group_id is not null and perm_''' + mode, (model_name, )) - groups = ', '.join(map(lambda x: x[0], cr.fetchall())) or '/' + groups = ', '.join(self.group_names_with_access(cr, model_name, mode)) or '/' msgs = { 'read': _("You can not read this document (%s) ! Be sure your user belongs to one of these groups: %s."), 'write': _("You can not write in this document (%s) ! Be sure your user belongs to one of these groups: %s."), From 444df6affa914a6dccd05ffaaeb02335488ec720 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Thu, 24 Mar 2011 12:17:57 +0100 Subject: [PATCH 03/17] [FIX] ir.rule,expression: domain expressions must be normalized before they can be safely combined This adds a normalize_domain() method to osv.expression, which ir.rule calls before combining expressions due to multiple rules or groups being applicable to a certain user. YAML tests are also added with a trivial unit test of normalize_domain() and some additional tests for ir_rule in order to verify that unnormalized domains are properly normalized before being combined by ir.rule. bzr revid: odo@openerp.com-20110324111757-uwuoqvm3lxkipr08 --- openerp/addons/base/ir/ir_rule.py | 10 ++-- openerp/addons/base/test/test_ir_rule.yml | 43 ++++++++++++++++- .../addons/base/test/test_osv_expression.yml | 12 +++++ openerp/osv/expression.py | 46 +++++++++++++++---- 4 files changed, 96 insertions(+), 15 deletions(-) diff --git a/openerp/addons/base/ir/ir_rule.py b/openerp/addons/base/ir/ir_rule.py index befa47e25b0..abf5744b140 100644 --- a/openerp/addons/base/ir/ir_rule.py +++ b/openerp/addons/base/ir/ir_rule.py @@ -19,7 +19,7 @@ # ############################################################################## -from osv import fields,osv +from osv import fields, osv, expression import time from operator import itemgetter from functools import partial @@ -37,7 +37,7 @@ class ir_rule(osv.osv): if rule.domain_force: eval_user_data = {'user': self.pool.get('res.users').browse(cr, 1, uid), 'time':time} - res[rule.id] = eval(rule.domain_force, eval_user_data) + res[rule.id] = expression.expression.normalize_domain(eval(rule.domain_force, eval_user_data)) else: res[rule.id] = [] return res @@ -91,7 +91,7 @@ class ir_rule(osv.osv): dom += rule.domain count += 1 if count: - return ['&'] * (count-1) + dom + return [expression.AND_OPERATOR] * (count-1) + dom return [] @tools.cache() @@ -127,9 +127,9 @@ class ir_rule(osv.osv): group_domains += group_domain count += 1 if count and global_domain: - return ['&'] + global_domain + ['|'] * (count-1) + group_domains + return [expression.AND_OPERATOR] + global_domain + [expression.OR_OPERATOR] * (count-1) + group_domains if count: - return ['|'] * (count-1) + group_domains + return [expression.OR_OPERATOR] * (count-1) + group_domains return global_domain return [] diff --git a/openerp/addons/base/test/test_ir_rule.yml b/openerp/addons/base/test/test_ir_rule.yml index 9352f798508..b5114cbedcb 100644 --- a/openerp/addons/base/test/test_ir_rule.yml +++ b/openerp/addons/base/test/test_ir_rule.yml @@ -132,6 +132,47 @@ !python {model: res.partner }: | ids = self.search(cr, ref('base.user_demo'), []) assert ids, "Demo user should see some partner." +- + Modify the ir_rule for employee so that demo is not + allowed to see partners anymore. We use a domain + with implicit AND operator for later tests on normalization. +- + !record {model: ir.rule, id: test_rule2}: + domain_force: "[('id','=',False),('name','=',False)]" +- + Check that demo user does not see partners anymore. +- + !python {model: res.partner }: | + ids = self.search(cr, ref('base.user_demo'), []) + assert (not ids), "Demo user should not see any partner anymore" +- + Create a new group with demo user in it, and a complex rule that should + re-allow demo to see partners, because he belongs to both groups. +- + !record {model: res.groups, id: test_group}: + name: Test Group + users: + - base.user_demo +- + Add the rule to the new group, with a domain containing an implicit AND operator, + which is more tricky because it will have to be normalized before combining it. +- + !record {model: ir.rule, id: test_rule3}: + model_id: base.model_res_partner + domain_force: "[('name', '!=', False),('id', '!=', False)]" + name: test_rule4 + groups: + - test_group + perm_unlink: 1 + perm_write: 1 + perm_read: 1 + perm_create: 1 +- + Read the partners again as demo user, which should give results again. +- + !python {model: res.partner }: | + ids = self.search(cr, ref('base.user_demo'), []) + assert ids, "Demo user should see some partners again, due to combined rules." - Delete global domains (to combine only group domains). - @@ -140,7 +181,7 @@ assert ids, "Demo user should see some partner." self.unlink(cr, uid, ids) - - Read as demo user the partners (three 1=1 domains, no global domain). + Read as demo user the partners (several group domains, no global domain). - !python {model: res.partner }: | ids = self.search(cr, ref('base.user_demo'), []) diff --git a/openerp/addons/base/test/test_osv_expression.yml b/openerp/addons/base/test/test_osv_expression.yml index 4916fe06203..61a686b3e35 100644 --- a/openerp/addons/base/test/test_osv_expression.yml +++ b/openerp/addons/base/test/test_osv_expression.yml @@ -110,4 +110,16 @@ res_ids = self.search(cr, uid, [('company_id.partner_id', 'not in', [])]) res_ids.sort() assert res_ids == all_ids, "Searching against empty set failed, returns %r" % res_ids +- + Verify that normalize_domain() works. +- + !python {model: res.partner}: | + from osv.expression import expression + norm_domain = domain = ['&',(1,'=',1),('a','=','b')] + assert norm_domain == expression.normalize_domain(domain), "Normalized domains should be left untouched" + domain = [('x','in',['y','z']),('a.v','=','e'),'|','|',('a','=','b'),'!',('c','>','d'),('e','!=','f'),('g','=','h')] + norm_domain = ['&','&','&'] + domain + assert norm_domain == expression.normalize_domain(domain), "Non-normalized domains should be properly normalized" + + diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index 1922d5e8388..0283935ea34 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -23,6 +23,9 @@ from openerp.tools import flatten, reverse_enumerate import fields +NOT_OPERATOR = '!' +OR_OPERATOR = '|' +AND_OPERATOR = '&' class expression(object): """ @@ -32,10 +35,12 @@ class expression(object): For more info: http://christophe-simonis-at-tiny.blogspot.com/2008/08/new-new-domain-notation.html """ - def _is_operator(self, element): - return isinstance(element, (str, unicode)) and element in ['&', '|', '!'] + @classmethod + def _is_operator(cls, element): + return isinstance(element, (str, unicode)) and element in [AND_OPERATOR, OR_OPERATOR, NOT_OPERATOR] - def _is_leaf(self, element, internal=False): + @classmethod + def _is_leaf(cls, element, internal=False): OPS = ('=', '!=', '<>', '<=', '<', '>', '>=', '=?', '=like', '=ilike', 'like', 'not like', 'ilike', 'not ilike', 'in', 'not in', 'child_of') INTERNAL_OPS = OPS + ('inselect',) return (isinstance(element, tuple) or isinstance(element, list)) \ @@ -43,6 +48,30 @@ class expression(object): and (((not internal) and element[1] in OPS) \ or (internal and element[1] in INTERNAL_OPS)) + @classmethod + def normalize_domain(cls, domain_expr): + """Returns a normalized version of ``domain_expr``, where all implicit '&' operators + have been made explicit. One property of normalized domain expressions is that they + can be easily combined together as if they were single domain components. + """ + assert isinstance(domain_expr, (list, tuple)), "Domain to normalize must have a 'domain' form: a list or tuple of domain components" + missing_operators = -1 + for item in domain_expr: + if cls._is_operator(item): + if item != NOT_OPERATOR: + missing_operators -= 1 + else: + missing_operators += 1 + return [AND_OPERATOR] * missing_operators + domain_expr + + def normalize(self): + """Make this expression normalized, i.e. change it so that all implicit '&' + operator become explicit. If the expression had already been parsed, + there is no need to do it again. + """ + self.__exp = expression.normalize_domain(self.__exp) + return self + def __execute_recursive_in(self, cr, s, f, w, ids, op, type): # todo: merge into parent query as sub-query res = [] @@ -92,8 +121,8 @@ class expression(object): doms = [] for o in table.browse(cr, uid, ids, context=context): if doms: - doms.insert(0, '|') - doms += ['&', ('parent_left', '<', o.parent_right), ('parent_left', '>=', o.parent_left)] + doms.insert(0, OR_OPERATOR) + doms += [AND_OPERATOR, ('parent_left', '<', o.parent_right), ('parent_left', '>=', o.parent_left)] if prefix: return [(left, 'in', table.search(cr, uid, doms, context=context))] return doms @@ -172,7 +201,7 @@ class expression(object): else: # we assume that the expression is valid # we create a dummy leaf for forcing the parsing of the resulting expression - self.__exp[i] = '&' + self.__exp[i] = AND_OPERATOR self.__exp.insert(i + 1, self.__DUMMY_LEAF) for j, se in enumerate(subexp): self.__exp.insert(i + 2 + j, se) @@ -387,7 +416,6 @@ class expression(object): ] self.__exp[i] = ('id', 'inselect', (query1, query2)) - return self def __leaf_to_sql(self, leaf, table): @@ -491,10 +519,10 @@ class expression(object): params.insert(0, p) stack.append(q) else: - if e == '!': + if e == NOT_OPERATOR: stack.append('(NOT (%s))' % (stack.pop(),)) else: - ops = {'&': ' AND ', '|': ' OR '} + ops = {AND_OPERATOR: ' AND ', OR_OPERATOR: ' OR '} q1 = stack.pop() q2 = stack.pop() stack.append('(%s %s %s)' % (q1, ops[e], q2,)) From 8ab9bac2b16659602932413be69141bba3e03ac2 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Fri, 8 Apr 2011 16:02:41 +0200 Subject: [PATCH 04/17] [IMP] ir.rule: added util methods for working with domain literals bzr revid: odo@openerp.com-20110408140241-jotm4lsr1savrr3a --- openerp/addons/base/ir/ir_rule.py | 39 ++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/openerp/addons/base/ir/ir_rule.py b/openerp/addons/base/ir/ir_rule.py index abf5744b140..149e51cd7c4 100644 --- a/openerp/addons/base/ir/ir_rule.py +++ b/openerp/addons/base/ir/ir_rule.py @@ -25,19 +25,52 @@ from operator import itemgetter from functools import partial import tools from tools.safe_eval import safe_eval as eval +from tools.misc import unquote as unquote class ir_rule(osv.osv): _name = 'ir.rule' _order = 'name' _MODES = ['read', 'write', 'create', 'unlink'] + def _eval_context_for_combinations(self): + """Returns a dictionary to use as evaluation context for + ir.rule domains, when the goal is to obtain python lists + that are easier to parse and combine, but not to + actually execute them.""" + return {'user': unquote('user'), + 'time': unquote('time')} + + def _eval_context(self, cr, uid): + """Returns a dictionary to use as evaluation context for + ir.rule domains.""" + return {'user': self.pool.get('res.users').browse(cr, 1, uid), + 'time':time} + + def domain_binary_operation(self, cr, uid, str_domain_1, str_domain_2, operator): + """Returns the string representation of the binary operation designated by + ``operator`` (should be '|' or '&') for the two string domains ``str_domain_1`` + and ``str_domain_2`` """ + eval_context = self._eval_context_for_combinations() + canonical_domain_1 = expression.expression.normalize_domain(eval(str_domain_1 or '[]', eval_context)) + canonical_domain_2 = expression.expression.normalize_domain(eval(str_domain_2 or '[]', eval_context)) + return str([operator] + canonical_domain_1 + canonical_domain_1) + + def domain_conjunction(self, cr, uid, str_domain_1, str_domain_2): + """Returns the string representation of the conjunction (AND) of the + two string domains ``str_domain_1`` and ``str_domain_2``""" + return self.domain_binary_operation(cr, uid, str_domain_1, str_domain_2, expression.AND_OPERATOR) + + def domain_disjunction(self, cr, uid, str_domain_1, str_domain_2): + """Returns the string representation of the disjunction (OR) of the + two string domains ``str_domain_1`` and ``str_domain_2``""" + return self.domain_binary_operation(cr, uid, str_domain_1, str_domain_2, expression.OR_OPERATOR) + def _domain_force_get(self, cr, uid, ids, field_name, arg, context=None): res = {} + eval_context = self._eval_context(cr, uid) for rule in self.browse(cr, uid, ids, context): if rule.domain_force: - eval_user_data = {'user': self.pool.get('res.users').browse(cr, 1, uid), - 'time':time} - res[rule.id] = expression.expression.normalize_domain(eval(rule.domain_force, eval_user_data)) + res[rule.id] = expression.expression.normalize_domain(eval(rule.domain_force, eval_context)) else: res[rule.id] = [] return res From c2623939500e31fd1df883a9a90ecc702e7043e0 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Mon, 11 Apr 2011 10:32:01 +0200 Subject: [PATCH 05/17] [IMP] tools.convert,rng: fixed support for tag bzr revid: odo@openerp.com-20110411083201-2ir0ed4o3xdal6tk --- openerp/import_xml.rng | 11 +++++++++++ openerp/tools/convert.py | 8 +++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/openerp/import_xml.rng b/openerp/import_xml.rng index 75c4e8b05e7..d301ae47ed3 100644 --- a/openerp/import_xml.rng +++ b/openerp/import_xml.rng @@ -188,6 +188,16 @@ + + + + + + + + + + @@ -222,6 +232,7 @@ + diff --git a/openerp/tools/convert.py b/openerp/tools/convert.py index 5536f311eb8..0841f0b94ac 100644 --- a/openerp/tools/convert.py +++ b/openerp/tools/convert.py @@ -403,7 +403,7 @@ form: module.record_id""" % (xml_id,) self._remove_ir_values(cr, string, value, model) def _tag_url(self, cr, rec, data_node=None): - url = rec.get("string",'').encode('utf8') + url = rec.get("url",'').encode('utf8') target = rec.get("target",'').encode('utf8') name = rec.get("name",'').encode('utf8') xml_id = rec.get('id','').encode('utf8') @@ -655,6 +655,12 @@ form: module.record_id""" % (xml_id,) resw = cr.fetchone() if (not values.get('name', False)) and resw: values['name'] = resw[0] + elif a_type=='url': + a_id = self.id_get(cr, a_action) + cr.execute('select name from ir_act_url where id=%s', (int(a_id),)) + resw = cr.fetchone() + if (not values.get('name')) and resw: + values['name'] = resw[0] if rec.get('sequence'): values['sequence'] = int(rec.get('sequence')) if rec.get('icon'): From 581d06ab65bb9394092ffe092186ca9b12a54a2a Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Tue, 31 May 2011 15:45:06 +0200 Subject: [PATCH 06/17] [FIX] res.partner.bank.type: everyone should be able to read bank account types bzr revid: odo@openerp.com-20110531134506-lkop1qey7vhhe56i --- openerp/addons/base/security/ir.model.access.csv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openerp/addons/base/security/ir.model.access.csv b/openerp/addons/base/security/ir.model.access.csv index 158d6d98694..9ff2c6aa0be 100644 --- a/openerp/addons/base/security/ir.model.access.csv +++ b/openerp/addons/base/security/ir.model.access.csv @@ -61,7 +61,7 @@ "access_res_partner_bank_group_user","res_partner_bank group_user","model_res_partner_bank","group_user",1,0,0,0 "access_res_partner_bank_group_partner_manager","res_partner_bank group_partner_manager","model_res_partner_bank","group_partner_manager",1,1,1,1 "access_res_partner_bank_type_group_partner_manager","res_partner_bank_type group_partner_manager","model_res_partner_bank_type","group_partner_manager",1,1,1,1 -"access_res_partner_bank_type_group_user","res_partner_bank_type group_user","model_res_partner_bank_type","group_user",1,0,0,0 +"access_res_partner_bank_type_group_user","res_partner_bank_type group_user","model_res_partner_bank_type",,1,0,0,0 "access_res_partner_bank_type_field_group_partner_manager","res_partner_bank_type_field group_partner_manager","model_res_partner_bank_type_field","group_partner_manager",1,1,1,1 "access_res_partner_bank_type_field_group_user","res_partner_bank_type_field group_user","model_res_partner_bank_type_field",,1,0,0,0 "access_res_partner_canal_group_user","res_partner_canal group_user","model_res_partner_canal","group_user",1,0,0,0 From e10e8073db1f254541e1f5a116f43bf92001a028 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Tue, 31 May 2011 16:43:18 +0200 Subject: [PATCH 07/17] [IMP] ir.rule,expression: merged improved rule computation mechanism, suggested by RCO bzr revid: odo@openerp.com-20110531144318-vut0opufvqd5mu07 --- openerp/addons/base/ir/ir_rule.py | 74 ++++++++------------------ openerp/osv/expression.py | 87 ++++++++++++++++++++++--------- 2 files changed, 85 insertions(+), 76 deletions(-) diff --git a/openerp/addons/base/ir/ir_rule.py b/openerp/addons/base/ir/ir_rule.py index 149e51cd7c4..52f76f2f611 100644 --- a/openerp/addons/base/ir/ir_rule.py +++ b/openerp/addons/base/ir/ir_rule.py @@ -27,6 +27,8 @@ import tools from tools.safe_eval import safe_eval as eval from tools.misc import unquote as unquote +SUPERUSER_UID = 1 + class ir_rule(osv.osv): _name = 'ir.rule' _order = 'name' @@ -46,31 +48,12 @@ class ir_rule(osv.osv): return {'user': self.pool.get('res.users').browse(cr, 1, uid), 'time':time} - def domain_binary_operation(self, cr, uid, str_domain_1, str_domain_2, operator): - """Returns the string representation of the binary operation designated by - ``operator`` (should be '|' or '&') for the two string domains ``str_domain_1`` - and ``str_domain_2`` """ - eval_context = self._eval_context_for_combinations() - canonical_domain_1 = expression.expression.normalize_domain(eval(str_domain_1 or '[]', eval_context)) - canonical_domain_2 = expression.expression.normalize_domain(eval(str_domain_2 or '[]', eval_context)) - return str([operator] + canonical_domain_1 + canonical_domain_1) - - def domain_conjunction(self, cr, uid, str_domain_1, str_domain_2): - """Returns the string representation of the conjunction (AND) of the - two string domains ``str_domain_1`` and ``str_domain_2``""" - return self.domain_binary_operation(cr, uid, str_domain_1, str_domain_2, expression.AND_OPERATOR) - - def domain_disjunction(self, cr, uid, str_domain_1, str_domain_2): - """Returns the string representation of the disjunction (OR) of the - two string domains ``str_domain_1`` and ``str_domain_2``""" - return self.domain_binary_operation(cr, uid, str_domain_1, str_domain_2, expression.OR_OPERATOR) - def _domain_force_get(self, cr, uid, ids, field_name, arg, context=None): res = {} eval_context = self._eval_context(cr, uid) for rule in self.browse(cr, uid, ids, context): if rule.domain_force: - res[rule.id] = expression.expression.normalize_domain(eval(rule.domain_force, eval_context)) + res[rule.id] = expression.normalize(eval(rule.domain_force, eval_context)) else: res[rule.id] = [] return res @@ -116,25 +99,12 @@ class ir_rule(osv.osv): (_check_model_obj, 'Rules are not supported for osv_memory objects !', ['model_id']) ] - def domain_create(self, cr, uid, rule_ids): - count = 0 - dom = [] - for rule in self.browse(cr, uid, rule_ids): - if rule.domain: - dom += rule.domain - count += 1 - if count: - return [expression.AND_OPERATOR] * (count-1) + dom - return [] - @tools.cache() def _compute_domain(self, cr, uid, model_name, mode="read"): if mode not in self._MODES: raise ValueError('Invalid mode: %r' % (mode,)) - group_rule = {} - global_rules = [] - if uid == 1: + if uid == SUPERUSER_UID: return None cr.execute("""SELECT r.id FROM ir_rule r @@ -144,26 +114,26 @@ class ir_rule(osv.osv): AND (r.id IN (SELECT rule_group_id FROM rule_group_rel g_rel JOIN res_groups_users_rel u_rel ON (g_rel.group_id = u_rel.gid) WHERE u_rel.uid = %s) OR r.global)""", (model_name, uid)) - ids = map(lambda x: x[0], cr.fetchall()) - if ids: - for rule in self.browse(cr, uid, ids): + rule_ids = [x[0] for x in cr.fetchall()] + if rule_ids: + # browse user as super-admin root to avoid access errors! + user = self.pool.get('res.users').browse(cr, SUPERUSER_UID, uid) + global_domains = [] # list of domains + group_domains = {} # map: group -> list of domains + for rule in self.browse(cr, SUPERUSER_UID, rule_ids): + dom = expression.normalize(rule.domain) for group in rule.groups: - group_rule.setdefault(group.id, []).append(rule.id) + if group in user.groups_id: + group_domains.setdefault(group, []).append(dom) if not rule.groups: - global_rules.append(rule.id) - global_domain = self.domain_create(cr, uid, global_rules) - count = 0 - group_domains = [] - for value in group_rule.values(): - group_domain = self.domain_create(cr, uid, value) - if group_domain: - group_domains += group_domain - count += 1 - if count and global_domain: - return [expression.AND_OPERATOR] + global_domain + [expression.OR_OPERATOR] * (count-1) + group_domains - if count: - return [expression.OR_OPERATOR] * (count-1) + group_domains - return global_domain + global_domains.append(dom) + # combine global domains and group domains + if group_domains: + group_domain = expression.OR(map(expression.AND, group_domains.values())) + else: + group_domain = [] + domain = expression.AND(global_domains + [group_domain]) + return domain return [] def clear_cache(self, cr, uid): diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index 0283935ea34..4b6a470ed55 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -27,6 +27,69 @@ NOT_OPERATOR = '!' OR_OPERATOR = '|' AND_OPERATOR = '&' +TRUE_DOMAIN = [(1,'=',1)] +FALSE_DOMAIN = [(0,'=',1)] + +def normalize(domain): + """Returns a normalized version of ``domain_expr``, where all implicit '&' operators + have been made explicit. One property of normalized domain expressions is that they + can be easily combined together as if they were single domain components. + """ + assert isinstance(domain, (list, tuple)), "Domains to normalize must have a 'domain' form: a list or tuple of domain components" + if not domain: + return TRUE_DOMAIN + result = [] + expected = 1 # expected number of expressions + op_arity = {NOT_OPERATOR: 1, AND_OPERATOR: 2, OR_OPERATOR: 2} + for token in domain: + if expected == 0: # more than expected, like in [A, B] + result[0:0] = ['&'] # put an extra '&' in front + expected = 1 + result.append(token) + if isinstance(token, (list,tuple)): # domain term + expected -= 1 + else: + expected += op_arity.get(token, 0) - 1 + assert expected == 0 + return result + +def combine(operator, unit, zero, domains): + """Returns a new domain expression where all domain components from ``domains`` + have been added together using the binary operator ``operator``. + + :param unit: the identity element of the domains "set" with regard to the operation + performed by ``operator``, i.e the domain component ``i`` which, when + combined with any domain ``x`` via ``operator``, yields ``x``. + E.g. [(1,'=',1)] is the typical unit for AND_OPERATOR: adding it + to any domain component gives the same domain. + :param zero: the absorbing element of the domains "set" with regard to the operation + performed by ``operator``, i.e the domain component ``z`` which, when + combined with any domain ``x`` via ``operator``, yields ``z``. + E.g. [(1,'=',1)] is the typical zero for OR_OPERATOR: as soon as + you see it in a domain component the resulting domain is the zero. + """ + result = [] + count = 0 + for domain in domains: + if domain == unit: + continue + if domain == zero: + return zero + if domain: + result += domain + count += 1 + result = [operator] * (count - 1) + result + return result + +def AND(domains): + """ AND([D1,D2,...]) returns a domain representing D1 and D2 and ... """ + return combine(AND_OPERATOR, TRUE_DOMAIN, FALSE_DOMAIN, domains) + +def OR(domains): + """ OR([D1,D2,...]) returns a domain representing D1 or D2 or ... """ + return combine(OR_OPERATOR, FALSE_DOMAIN, TRUE_DOMAIN, domains) + + class expression(object): """ parse a domain expression @@ -48,30 +111,6 @@ class expression(object): and (((not internal) and element[1] in OPS) \ or (internal and element[1] in INTERNAL_OPS)) - @classmethod - def normalize_domain(cls, domain_expr): - """Returns a normalized version of ``domain_expr``, where all implicit '&' operators - have been made explicit. One property of normalized domain expressions is that they - can be easily combined together as if they were single domain components. - """ - assert isinstance(domain_expr, (list, tuple)), "Domain to normalize must have a 'domain' form: a list or tuple of domain components" - missing_operators = -1 - for item in domain_expr: - if cls._is_operator(item): - if item != NOT_OPERATOR: - missing_operators -= 1 - else: - missing_operators += 1 - return [AND_OPERATOR] * missing_operators + domain_expr - - def normalize(self): - """Make this expression normalized, i.e. change it so that all implicit '&' - operator become explicit. If the expression had already been parsed, - there is no need to do it again. - """ - self.__exp = expression.normalize_domain(self.__exp) - return self - def __execute_recursive_in(self, cr, s, f, w, ids, op, type): # todo: merge into parent query as sub-query res = [] From bf0e9e923ce8b4f2f2c8c3a8d95dd5b42db23e05 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Wed, 1 Jun 2011 07:50:31 +0200 Subject: [PATCH 08/17] [FIX] osv_memory: load config params in __init__ to avoid uninitialized values bzr revid: odo@openerp.com-20110601055031-xyungliji7snspsj --- openerp/osv/orm.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/openerp/osv/orm.py b/openerp/osv/orm.py index 5197f92f3d0..1320ee3c6dc 100644 --- a/openerp/osv/orm.py +++ b/openerp/osv/orm.py @@ -2054,8 +2054,8 @@ class orm_memory(orm_template): _protected = ['read', 'write', 'create', 'default_get', 'perm_read', 'unlink', 'fields_get', 'fields_view_get', 'search', 'name_get', 'distinct_field_get', 'name_search', 'copy', 'import_data', 'search_count', 'exists'] _inherit_fields = {} - _max_count = config.get('osv_memory_count_limit') - _max_hours = config.get('osv_memory_age_limit') + _max_count = None + _max_hours = None _check_time = 20 @classmethod @@ -2067,6 +2067,8 @@ class orm_memory(orm_template): self.datas = {} self.next_id = 0 self.check_id = 0 + self._max_count = config.get('osv_memory_count_limit') + self._max_hours = config.get('osv_memory_age_limit') cr.execute('delete from wkf_instance where res_type=%s', (self._name,)) def _check_access(self, uid, object_id, mode): From cf09b1df94afb57be723ec59fa16e533c5ac49d7 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Wed, 1 Jun 2011 08:23:03 +0200 Subject: [PATCH 09/17] [FIX] test_osv_expression: fixed for new normalization API bzr revid: odo@openerp.com-20110601062303-k9z0birutesfmkhp --- openerp/addons/base/test/test_osv_expression.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openerp/addons/base/test/test_osv_expression.yml b/openerp/addons/base/test/test_osv_expression.yml index 61a686b3e35..04177c8297f 100644 --- a/openerp/addons/base/test/test_osv_expression.yml +++ b/openerp/addons/base/test/test_osv_expression.yml @@ -114,12 +114,12 @@ Verify that normalize_domain() works. - !python {model: res.partner}: | - from osv.expression import expression + from osv import expression norm_domain = domain = ['&',(1,'=',1),('a','=','b')] - assert norm_domain == expression.normalize_domain(domain), "Normalized domains should be left untouched" + assert norm_domain == expression.normalize(domain), "Normalized domains should be left untouched" domain = [('x','in',['y','z']),('a.v','=','e'),'|','|',('a','=','b'),'!',('c','>','d'),('e','!=','f'),('g','=','h')] norm_domain = ['&','&','&'] + domain - assert norm_domain == expression.normalize_domain(domain), "Non-normalized domains should be properly normalized" + assert norm_domain == expression.normalize(domain), "Non-normalized domains should be properly normalized" From 6bbe8c7200a1602de3e28d9765b1dd420383a355 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Wed, 1 Jun 2011 15:01:56 +0200 Subject: [PATCH 10/17] [IMP] ir.rule: local rules will be combined with OR within the same group This makes it more intuitive: global rules are restrictions and always combined with AND, while local rules are always combined with OR, regardless if they are in the same group or different groups. It makes it also easier to add rules in an existing group to grant more rights. bzr revid: odo@openerp.com-20110601130156-c0swo9w21zqun2gl --- openerp/addons/base/ir/ir.xml | 23 ++++++++++++++--------- openerp/addons/base/ir/ir_rule.py | 2 +- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/openerp/addons/base/ir/ir.xml b/openerp/addons/base/ir/ir.xml index 8b39ac925b2..16630cbd40f 100644 --- a/openerp/addons/base/ir/ir.xml +++ b/openerp/addons/base/ir/ir.xml @@ -1667,24 +1667,29 @@ - + - + - - diff --git a/openerp/addons/base/ir/ir_rule.py b/openerp/addons/base/ir/ir_rule.py index 52f76f2f611..d9da5ac6924 100644 --- a/openerp/addons/base/ir/ir_rule.py +++ b/openerp/addons/base/ir/ir_rule.py @@ -129,7 +129,7 @@ class ir_rule(osv.osv): global_domains.append(dom) # combine global domains and group domains if group_domains: - group_domain = expression.OR(map(expression.AND, group_domains.values())) + group_domain = expression.OR(map(expression.OR, group_domains.values())) else: group_domain = [] domain = expression.AND(global_domains + [group_domain]) From ce029f887e04f5fd79dbecec93f4258df2b63c8c Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Wed, 1 Jun 2011 15:55:26 +0200 Subject: [PATCH 11/17] [FIX] ir.rule: must eval the rule domain as user, but browse the rule's groups as admin bzr revid: odo@openerp.com-20110601135526-gk7zfiylnhlpvlod --- openerp/addons/base/ir/ir_rule.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/openerp/addons/base/ir/ir_rule.py b/openerp/addons/base/ir/ir_rule.py index d9da5ac6924..2a36ee9ccc5 100644 --- a/openerp/addons/base/ir/ir_rule.py +++ b/openerp/addons/base/ir/ir_rule.py @@ -121,7 +121,9 @@ class ir_rule(osv.osv): global_domains = [] # list of domains group_domains = {} # map: group -> list of domains for rule in self.browse(cr, SUPERUSER_UID, rule_ids): - dom = expression.normalize(rule.domain) + # read 'domain' as UID to have the correct eval context for the rule. + rule_domain = self.read(cr, uid, rule.id, ['domain'])['domain'] + dom = expression.normalize(rule_domain) for group in rule.groups: if group in user.groups_id: group_domains.setdefault(group, []).append(dom) From 26f73ce7e8da325f8b64562608174aa1bb6331f9 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Wed, 1 Jun 2011 16:39:21 +0200 Subject: [PATCH 12/17] [IMP] base: update ir.rule tests for new combination semantics (Rules from the same local group are now OR'ed, instead of AND'ed. bzr revid: odo@openerp.com-20110601143921-g3tv46krsa01y4tv --- openerp/addons/base/test/test_ir_rule.yml | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/openerp/addons/base/test/test_ir_rule.yml b/openerp/addons/base/test/test_ir_rule.yml index b5114cbedcb..49b368a5bb3 100644 --- a/openerp/addons/base/test/test_ir_rule.yml +++ b/openerp/addons/base/test/test_ir_rule.yml @@ -133,21 +133,22 @@ ids = self.search(cr, ref('base.user_demo'), []) assert ids, "Demo user should see some partner." - - Modify the ir_rule for employee so that demo is not - allowed to see partners anymore. We use a domain - with implicit AND operator for later tests on normalization. + Modify the ir_rule for employee to have a rule that fordids + seeing any record. + We use a domain with implicit AND operator for later tests + on normalization. - !record {model: ir.rule, id: test_rule2}: domain_force: "[('id','=',False),('name','=',False)]" - - Check that demo user does not see partners anymore. + Check that demo user still sees partners, because group-rules are OR'ed. - !python {model: res.partner }: | ids = self.search(cr, ref('base.user_demo'), []) - assert (not ids), "Demo user should not see any partner anymore" + assert ids, "Demo user should see some partner." - - Create a new group with demo user in it, and a complex rule that should - re-allow demo to see partners, because he belongs to both groups. + Create a new group with demo user in it, and a complex rule. + Demo should still see partners. - !record {model: res.groups, id: test_group}: name: Test Group @@ -168,11 +169,11 @@ perm_read: 1 perm_create: 1 - - Read the partners again as demo user, which should give results again. + Read the partners again as demo user, which should give results. - !python {model: res.partner }: | ids = self.search(cr, ref('base.user_demo'), []) - assert ids, "Demo user should see some partners again, due to combined rules." + assert ids, "Demo user should see partners even with the combined rules." - Delete global domains (to combine only group domains). - From 6df3bc8b6773bdb5741ea314eae6f72dfb08e95e Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Sun, 19 Jun 2011 13:54:10 +0200 Subject: [PATCH 13/17] [FIX] orm.fields_get: typo in last refactoring bzr revid: odo@openerp.com-20110619115410-bdmd4zcm3o69515o --- openerp/osv/orm.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openerp/osv/orm.py b/openerp/osv/orm.py index 893035b6ad8..2b52b70971f 100644 --- a/openerp/osv/orm.py +++ b/openerp/osv/orm.py @@ -1286,8 +1286,8 @@ class orm_template(object): res[f] = fields.field_to_dict(self, cr, user, context, field) if not write_access: - res['readonly'] = True - res['states'] = {} + res[f]['readonly'] = True + res[f]['states'] = {} if hasattr(res[f], 'string'): res_trans = translation_obj._get_source(cr, user, self._name + ',' + f, 'field', context.get('lang', False) or 'en_US') From dd1cefe44656340d01f8fd1a476ec9b1d6a11825 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Sun, 19 Jun 2011 14:40:42 +0200 Subject: [PATCH 14/17] [FIX] res_user: typo when deleting group with more than 4 users bzr revid: odo@openerp.com-20110619124042-qjoqges5b5jpxr59 --- openerp/addons/base/res/res_user.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/openerp/addons/base/res/res_user.py b/openerp/addons/base/res/res_user.py index 8a76ba9a6f4..55451f1bb50 100644 --- a/openerp/addons/base/res/res_user.py +++ b/openerp/addons/base/res/res_user.py @@ -582,8 +582,7 @@ class groups2(osv.osv): ##FIXME: Is there a reason to inherit this object ? if group_users: user_names = [user.name for user in self.pool.get('res.users').browse(cr, uid, group_users, context=context)] if len(user_names) >= 5: - user_names = user_names[:5] - user_names += '...' + user_names = user_names[:5] + ['...'] raise osv.except_osv(_('Warning !'), _('Group(s) cannot be deleted, because some user(s) still belong to them: %s !') % \ ', '.join(user_names)) From c46fa82418def79cbfea8796040be569f894ab94 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Sun, 19 Jun 2011 15:09:39 +0200 Subject: [PATCH 15/17] [FIX] res_user: de-duplicate usernames for warning msg about group deletion bzr revid: odo@openerp.com-20110619130939-nnwi3u2ohl150mr6 --- openerp/addons/base/res/res_user.py | 1 + 1 file changed, 1 insertion(+) diff --git a/openerp/addons/base/res/res_user.py b/openerp/addons/base/res/res_user.py index 55451f1bb50..50d9c526957 100644 --- a/openerp/addons/base/res/res_user.py +++ b/openerp/addons/base/res/res_user.py @@ -581,6 +581,7 @@ class groups2(osv.osv): ##FIXME: Is there a reason to inherit this object ? if group_users: user_names = [user.name for user in self.pool.get('res.users').browse(cr, uid, group_users, context=context)] + user_names = list(set(user_names)) if len(user_names) >= 5: user_names = user_names[:5] + ['...'] raise osv.except_osv(_('Warning !'), From 66b4f6fbcd89235d46339b8f1406646f52abb1c8 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Sun, 19 Jun 2011 19:11:02 +0200 Subject: [PATCH 16/17] [FIX] fields: function/property fields should perform their name_get() calls for m2o as uid 1 This is because the permissions for reading the display name of a m2o record does not depend on access to the target table, but depends on the user access to the current table. Users that are denied read access to the target table may still see the names of the records linked to the documents they can read bzr revid: odo@openerp.com-20110619171102-sh0derdj50epea7b --- openerp/osv/fields.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/openerp/osv/fields.py b/openerp/osv/fields.py index f55e4b9c027..e28f7b2b02d 100644 --- a/openerp/osv/fields.py +++ b/openerp/osv/fields.py @@ -936,6 +936,9 @@ class related(function): if self._type=='many2one': ids = filter(None, res.values()) if ids: + # name_get as root, as seeing the name of a related + # object depends on access right of source document, + # not target, so user may not have access. ng = dict(obj.pool.get(self._obj).name_get(cr, 1, ids, context=context)) for r in res: if res[r]: @@ -1079,7 +1082,10 @@ class property(function): value = properties.get_by_record(cr, uid, prop, context=context) res[prop.res_id.id][prop.fields_id.name] = value or False if value and (prop.type == 'many2one'): - record_exists = obj.pool.get(value._name).exists(cr, uid, value.id) + # check existence as root, as seeing the name of a related + # object depends on access right of source document, + # not target, so user may not have access. + record_exists = obj.pool.get(value._name).exists(cr, 1, value.id) if record_exists: replaces.setdefault(value._name, {}) replaces[value._name][value.id] = True @@ -1087,8 +1093,11 @@ class property(function): res[prop.res_id.id][prop.fields_id.name] = False for rep in replaces: - nids = obj.pool.get(rep).search(cr, uid, [('id','in',replaces[rep].keys())], context=context) - replaces[rep] = dict(obj.pool.get(rep).name_get(cr, uid, nids, context=context)) + # search+name_get as root, as seeing the name of a related + # object depends on access right of source document, + # not target, so user may not have access. + nids = obj.pool.get(rep).search(cr, 1, [('id','in',replaces[rep].keys())], context=context) + replaces[rep] = dict(obj.pool.get(rep).name_get(cr, 1, nids, context=context)) for prop in prop_name: for id in ids: From 301497c5432ecba27e4c7615beeaffea3b549b7c Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Fri, 24 Jun 2011 15:33:35 +0200 Subject: [PATCH 17/17] [IMP] lint cleanup bzr revid: odo@openerp.com-20110624133335-jf5kg6dh9kgxw2dg --- openerp/addons/base/ir/ir_model.py | 1 - openerp/addons/base/ir/ir_ui_view.py | 1 - openerp/netsvc.py | 2 -- openerp/osv/orm.py | 2 +- openerp/tools/convert.py | 1 - openerp/tools/yaml_import.py | 3 +-- 6 files changed, 2 insertions(+), 8 deletions(-) diff --git a/openerp/addons/base/ir/ir_model.py b/openerp/addons/base/ir/ir_model.py index 4b482085ad6..d3dfd987638 100644 --- a/openerp/addons/base/ir/ir_model.py +++ b/openerp/addons/base/ir/ir_model.py @@ -21,7 +21,6 @@ import logging import re import time -from operator import itemgetter from osv import fields,osv import netsvc diff --git a/openerp/addons/base/ir/ir_ui_view.py b/openerp/addons/base/ir/ir_ui_view.py index ebbfe7cea38..ee23a2f52f1 100644 --- a/openerp/addons/base/ir/ir_ui_view.py +++ b/openerp/addons/base/ir/ir_ui_view.py @@ -24,7 +24,6 @@ from lxml import etree from tools import graph from tools.safe_eval import safe_eval as eval import tools -import netsvc import os import logging diff --git a/openerp/netsvc.py b/openerp/netsvc.py index aca1b9c12a9..1d9e0a5fcc0 100644 --- a/openerp/netsvc.py +++ b/openerp/netsvc.py @@ -35,7 +35,6 @@ import socket import sys import threading import time -import warnings import types from pprint import pformat @@ -186,7 +185,6 @@ class ColoredFormatter(DBFormatter): return DBFormatter.format(self, record) def init_logger(): - import os from tools.translate import resetlocale resetlocale() diff --git a/openerp/osv/orm.py b/openerp/osv/orm.py index be7045e2f11..a817f73310d 100644 --- a/openerp/osv/orm.py +++ b/openerp/osv/orm.py @@ -972,7 +972,7 @@ class orm_template(object): if field[len(prefix)]=='id': try: data_res_id = _get_id(model_name, line[i], current_module, 'id') - except ValueError, e: + except ValueError: pass xml_id = line[i] continue diff --git a/openerp/tools/convert.py b/openerp/tools/convert.py index 79906a008d2..77abfa572b4 100644 --- a/openerp/tools/convert.py +++ b/openerp/tools/convert.py @@ -45,7 +45,6 @@ import openerp.loglevels as loglevels import openerp.pooler as pooler from config import config from translate import _ -from yaml_import import convert_yaml_import # List of etree._Element subclasses that we choose to ignore when parsing XML. from misc import SKIPPED_ELEMENT_TYPES diff --git a/openerp/tools/yaml_import.py b/openerp/tools/yaml_import.py index 1c6a54c63b0..00bb79f0499 100644 --- a/openerp/tools/yaml_import.py +++ b/openerp/tools/yaml_import.py @@ -177,7 +177,7 @@ class YamlInterpreter(object): try: _, id = self.pool.get('ir.model.data').get_object_reference(self.cr, self.uid, module, checked_xml_id) self.id_map[xml_id] = id - except ValueError, e: + except ValueError: raise ValueError("""%s not found when processing %s. This Yaml file appears to depend on missing data. This often happens for tests that belong to a module's test suite and depend on each other.""" % (checked_xml_id, self.filename)) @@ -488,7 +488,6 @@ class YamlInterpreter(object): if self.isnoupdate(function) and self.mode != 'init': return model = self.get_model(function.model) - context = self.get_context(function, self.eval_context) if function.eval: args = self.process_eval(function.eval) else: