diff --git a/openerp/addons/base/res/res_partner_view.xml b/openerp/addons/base/res/res_partner_view.xml index caa5ce22a6d..81d9c00f120 100644 --- a/openerp/addons/base/res/res_partner_view.xml +++ b/openerp/addons/base/res/res_partner_view.xml @@ -211,7 +211,7 @@
-
+
@@ -298,7 +298,7 @@ - + diff --git a/openerp/addons/base/res/res_users.py b/openerp/addons/base/res/res_users.py index a1c06a52494..14673ce118f 100644 --- a/openerp/addons/base/res/res_users.py +++ b/openerp/addons/base/res/res_users.py @@ -409,8 +409,8 @@ class res_users(osv.osv): # prevent/delay login in that case. It will also have been logged # as a SQL error, if anyone cares. try: - cr.execute("SELECT id FROM res_users WHERE id=%s FOR UPDATE NOWAIT", str(user_id)) - cr.execute("UPDATE res_users SET login_date = now() AT TIME ZONE 'UTC' WHERE id=%s", str(user_id)) + cr.execute("SELECT id FROM res_users WHERE id=%s FOR UPDATE NOWAIT", (user_id,)) + cr.execute("UPDATE res_users SET login_date = now() AT TIME ZONE 'UTC' WHERE id=%s", (user_id,)) except Exception, e: _logger.exception("Failed to update last_login for db:%s login:%s", db, login) except openerp.exceptions.AccessDenied: diff --git a/openerp/addons/base/test/test_osv_expression.yml b/openerp/addons/base/test/test_osv_expression.yml index 3843d374e64..e2a104bfff2 100644 --- a/openerp/addons/base/test/test_osv_expression.yml +++ b/openerp/addons/base/test/test_osv_expression.yml @@ -396,7 +396,7 @@ assert res_8 == [1] assert res_9 == [] - # get the companies referenced by some currency (this is normally the main company) + # get the companies referenced by some currency (this is normally the main company) using a weird negative domain res_10 = self.search(cr, uid, [('currency_ids', 'not like', 'probably_unexisting_name')]) res_11 = self.search(cr, uid, [('currency_ids', 'not in', [max_currency_id + 1])]) res_12 = self.search(cr, uid, [('currency_ids', '!=', False)]) @@ -408,6 +408,20 @@ assert res_10 == res_11 assert res_10 == res_12 assert res_10 == res_13 + + # try testing real subsets with IN/NOT IN + res_partner = self.pool.get('res.partner') + res_users = self.pool.get('res.users') + p1, _ = res_partner.name_create(cr, uid, "Dédé Boitaclou") + p2, _ = res_partner.name_create(cr, uid, "Raoulette Pizza O'poil") + u1a = res_users.create(cr, uid, {'login': 'dbo', 'partner_id': p1}) + u1b = res_users.create(cr, uid, {'login': 'dbo2', 'partner_id': p1}) + u2 = res_users.create(cr, uid, {'login': 'rpo', 'partner_id': p2}) + assert [p1] == res_partner.search(cr, uid, [('user_ids', 'in', u1a)]), "o2m IN accept single int on right side" + assert [p1,p2] == res_partner.search(cr, uid, [('user_ids', 'in', [u1a,u2])]), "o2m IN matches any on the right side" + all_partners = res_partner.search(cr, uid, []) + assert (set(all_partners) - set([p1])) == set(res_partner.search(cr, uid, [('user_ids', 'not in', u1a)])), "o2m NOT IN matches none on the right side" + assert (set(all_partners) - set([p1,p2])) == set(res_partner.search(cr, uid, [('user_ids', 'not in', [u1b, u2])])), "o2m NOT IN matches none on the right side" # child_of x returns x and its children (direct or not). company = self.browse(cr, uid, ref('ymltest_company3')) diff --git a/openerp/osv/expression.py b/openerp/osv/expression.py index 5f2c0b915c1..2b2d33a496e 100644 --- a/openerp/osv/expression.py +++ b/openerp/osv/expression.py @@ -540,7 +540,8 @@ class expression(object): ids2 = select_from_where(cr, field._fields_id, field_obj._table, 'id', ids2, operator) if ids2: call_null = False - self.__exp[i] = ('id', 'in', ids2) + o2m_op = 'not in' if operator in NEGATIVE_TERM_OPERATORS else 'in' + self.__exp[i] = ('id', o2m_op, ids2) if call_null: o2m_op = 'in' if operator in NEGATIVE_TERM_OPERATORS else 'not in' diff --git a/openerp/sql_db.py b/openerp/sql_db.py index a6ebcd0300f..f18414bdff8 100644 --- a/openerp/sql_db.py +++ b/openerp/sql_db.py @@ -215,6 +215,9 @@ class Cursor(object): _logger.warning(query) _logger.warning("SQL queries cannot contain %d or %f anymore. " "Use only %s") + if params and not isinstance(params, (tuple, list, dict)): + _logger.error("SQL query parameters should be a tuple, list or dict; got %r", params) + raise ValueError("SQL query parameters should be a tuple, list or dict; got %r" % (params,)) if self.sql_log: now = mdt.now() diff --git a/openerp/tests/__init__.py b/openerp/tests/__init__.py index aa3055f471c..3427743263c 100644 --- a/openerp/tests/__init__.py +++ b/openerp/tests/__init__.py @@ -9,7 +9,7 @@ See the :ref:`test-framework` section in the :ref:`features` list. """ from . import test_expression, test_html_sanitize, test_ir_sequence, test_orm,\ - test_view_validation, test_uninstall, test_misc + test_view_validation, test_uninstall, test_misc, test_db_cursor fast_suite = [ test_ir_sequence, @@ -18,6 +18,7 @@ fast_suite = [ checks = [ test_expression, test_html_sanitize, + test_db_cursor, test_orm, test_view_validation, test_misc, diff --git a/openerp/tests/test_db_cursor.py b/openerp/tests/test_db_cursor.py new file mode 100644 index 00000000000..e9737547619 --- /dev/null +++ b/openerp/tests/test_db_cursor.py @@ -0,0 +1,32 @@ +# -*- coding: utf-8 -*- + +import unittest2 + +import openerp +from openerp.tools.misc import mute_logger +import common + +DB = common.DB +ADMIN_USER_ID = common.ADMIN_USER_ID + +def cursor(): + return openerp.modules.registry.RegistryManager.get(DB).db.cursor() + + +class test_cr_execute(unittest2.TestCase): + """ Try cr.execute with wrong parameters """ + + @mute_logger('openerp.sql_db') + def test_execute_bad_params(self): + """ + Try to use iterable but non-list or int params in query parameters. + """ + cr = cursor() + with self.assertRaises(ValueError): + cr.execute("SELECT id FROM res_users WHERE login=%s", 'admin') + with self.assertRaises(ValueError): + cr.execute("SELECT id FROM res_users WHERE id=%s", 1) + with self.assertRaises(ValueError): + cr.execute("SELECT id FROM res_users WHERE id=%s", '1') + +# vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4: diff --git a/openerp/tools/cache.py b/openerp/tools/cache.py index 2c5c4a46e0d..6e18007c340 100644 --- a/openerp/tools/cache.py +++ b/openerp/tools/cache.py @@ -43,7 +43,7 @@ class ormcache(object): return r except KeyError: self.stat_miss += 1 - value = d[args] = self.method(self2, cr, *args) + value = d[key] = self.method(self2, cr, *args) return value except TypeError: self.stat_err += 1 diff --git a/openerp/tools/safe_eval.py b/openerp/tools/safe_eval.py index 601fd95200a..1c3870004a7 100644 --- a/openerp/tools/safe_eval.py +++ b/openerp/tools/safe_eval.py @@ -46,7 +46,7 @@ _ALLOWED_MODULES = ['_strptime', 'time'] _CONST_OPCODES = set(opmap[x] for x in [ 'POP_TOP', 'ROT_TWO', 'ROT_THREE', 'ROT_FOUR', 'DUP_TOP', 'DUP_TOPX', 'POP_BLOCK','SETUP_LOOP', 'BUILD_LIST', 'BUILD_MAP', 'BUILD_TUPLE', - 'LOAD_CONST', 'RETURN_VALUE', 'STORE_SUBSCR'] if x in opmap) + 'LOAD_CONST', 'RETURN_VALUE', 'STORE_SUBSCR', 'STORE_MAP'] if x in opmap) _EXPR_OPCODES = _CONST_OPCODES.union(set(opmap[x] for x in [ 'UNARY_POSITIVE', 'UNARY_NEGATIVE', 'UNARY_NOT', @@ -61,7 +61,7 @@ _EXPR_OPCODES = _CONST_OPCODES.union(set(opmap[x] for x in [ ] if x in opmap)) _SAFE_OPCODES = _EXPR_OPCODES.union(set(opmap[x] for x in [ - 'STORE_MAP', 'LOAD_NAME', 'CALL_FUNCTION', 'COMPARE_OP', 'LOAD_ATTR', + 'LOAD_NAME', 'CALL_FUNCTION', 'COMPARE_OP', 'LOAD_ATTR', 'STORE_NAME', 'GET_ITER', 'FOR_ITER', 'LIST_APPEND', 'DELETE_NAME', 'JUMP_FORWARD', 'JUMP_IF_TRUE', 'JUMP_IF_FALSE', 'JUMP_ABSOLUTE', 'MAKE_FUNCTION', 'SLICE+0', 'SLICE+1', 'SLICE+2', 'SLICE+3',