diff --git a/openerp/addons/base/res/res_users.py b/openerp/addons/base/res/res_users.py index 772f7f7290b..32e81ee600f 100644 --- a/openerp/addons/base/res/res_users.py +++ b/openerp/addons/base/res/res_users.py @@ -388,14 +388,13 @@ class res_users(osv.osv): if not password: return False user_id = False - cr = self.pool.db.cursor() + cr = self.pool.cursor() try: # autocommit: our single update request will be performed atomically. # (In this way, there is no opportunity to have two transactions # interleaving their cr.execute()..cr.commit() calls and have one # of them rolled back due to a concurrent access.) - if not openerp.tools.config['test_enable']: - cr.autocommit(True) + cr.autocommit(True) # check if user exists res = self.search(cr, SUPERUSER_ID, [('login','=',login)]) if res: @@ -440,7 +439,7 @@ class res_users(osv.osv): # Successfully logged in as admin! # Attempt to guess the web base url... if user_agent_env and user_agent_env.get('base_location'): - cr = self.pool.db.cursor() + cr = self.pool.cursor() try: base = user_agent_env['base_location'] ICP = self.pool['ir.config_parameter'] @@ -461,7 +460,7 @@ class res_users(osv.osv): raise openerp.exceptions.AccessDenied() if self._uid_cache.get(db, {}).get(uid) == passwd: return - cr = self.pool.db.cursor() + cr = self.pool.cursor() try: self.check_credentials(cr, uid, passwd) if self._uid_cache.has_key(db): diff --git a/openerp/addons/base/tests/test_db_cursor.py b/openerp/addons/base/tests/test_db_cursor.py index b3ce36467bb..d60f38185b9 100644 --- a/openerp/addons/base/tests/test_db_cursor.py +++ b/openerp/addons/base/tests/test_db_cursor.py @@ -21,7 +21,7 @@ class test_cr_execute(unittest2.TestCase): """ Try to use iterable but non-list or int params in query parameters. """ - with registry().cursor(auto_commit=False) as cr: + with registry().cursor() as cr: with self.assertRaises(ValueError): cr.execute("SELECT id FROM res_users WHERE login=%s", 'admin') with self.assertRaises(ValueError): diff --git a/openerp/addons/base/tests/test_ir_sequence.py b/openerp/addons/base/tests/test_ir_sequence.py index e925fffa3b1..18b56daa58c 100644 --- a/openerp/addons/base/tests/test_ir_sequence.py +++ b/openerp/addons/base/tests/test_ir_sequence.py @@ -21,7 +21,7 @@ def registry(model): return openerp.modules.registry.RegistryManager.get(DB)[model] def cursor(): - return openerp.modules.registry.RegistryManager.get(DB).db.cursor() + return openerp.modules.registry.RegistryManager.get(DB).cursor() def drop_sequence(code): diff --git a/openerp/addons/base/tests/test_uninstall.py b/openerp/addons/base/tests/test_uninstall.py index 2425249afa4..b5c51c81405 100644 --- a/openerp/addons/base/tests/test_uninstall.py +++ b/openerp/addons/base/tests/test_uninstall.py @@ -13,7 +13,7 @@ def registry(model): return openerp.modules.registry.RegistryManager.get(DB)[model] def cursor(): - return openerp.modules.registry.RegistryManager.get(DB).db.cursor() + return openerp.modules.registry.RegistryManager.get(DB).cursor() def get_module(module_name): registry = openerp.modules.registry.RegistryManager.get(DB) diff --git a/openerp/addons/base/tests/test_views.py b/openerp/addons/base/tests/test_views.py index 06030a84792..f0967836726 100644 --- a/openerp/addons/base/tests/test_views.py +++ b/openerp/addons/base/tests/test_views.py @@ -28,7 +28,7 @@ class ViewCase(common.TransactionCase): self.assertTreesEqual(c1, c2, msg) -class TestNodeLocator(common.BaseCase): +class TestNodeLocator(common.TransactionCase): """ The node locator returns None when it can not find a node, and the first match when it finds something (no jquery-style node sets) diff --git a/openerp/cli/server.py b/openerp/cli/server.py index 561b9a19879..18ac69c38e2 100644 --- a/openerp/cli/server.py +++ b/openerp/cli/server.py @@ -111,7 +111,7 @@ def export_translation(): fileformat = os.path.splitext(config["translate_out"])[-1][1:].lower() buf = file(config["translate_out"], "w") registry = openerp.modules.registry.RegistryManager.new(dbname) - cr = registry.db.cursor() + cr = registry.cursor() openerp.tools.trans_export(config["language"], config["translate_modules"] or ["all"], buf, fileformat, cr) cr.close() @@ -125,7 +125,7 @@ def import_translation(): dbname = config['db_name'] registry = openerp.modules.registry.RegistryManager.new(dbname) - cr = registry.db.cursor() + cr = registry.cursor() openerp.tools.trans_load( cr, config["translate_in"], config["language"], context=context) cr.commit() diff --git a/openerp/http.py b/openerp/http.py index 2e21d27528a..8d7497fc40d 100644 --- a/openerp/http.py +++ b/openerp/http.py @@ -235,10 +235,7 @@ class WebRequest(object): """ # some magic to lazy create the cr if not self._cr: - # Test cursors - self._cr = openerp.tests.common.acquire_test_cursor(self.session_id) - if not self._cr: - self._cr = self.registry.db.cursor() + self._cr = self.registry.cursor() return self._cr def __enter__(self): @@ -249,14 +246,9 @@ class WebRequest(object): _request_stack.pop() if self._cr: - # Dont close test cursors - if not openerp.tests.common.release_test_cursor(self._cr): - if exc_type is None and not self._failed: - self._cr.commit() - else: - # just to be explicit - happens at close() anyway - self._cr.rollback() - self._cr.close() + if exc_type is None and not self._failed: + self._cr.commit() + self._cr.close() # just to be sure no one tries to re-use the request self.disable_db = True self.uid = None @@ -294,7 +286,7 @@ class WebRequest(object): def checked_call(___dbname, *a, **kw): # The decorator can call us more than once if there is an database error. In this # case, the request cursor is unusable. Rollback transaction to create a new one. - if self._cr and not openerp.tools.config['test_enable']: + if self._cr: self._cr.rollback() return self.endpoint(*a, **kw) diff --git a/openerp/modules/registry.py b/openerp/modules/registry.py index 78b7e5caeab..ef6aaf19d5a 100644 --- a/openerp/modules/registry.py +++ b/openerp/modules/registry.py @@ -58,7 +58,10 @@ class Registry(Mapping): self._init_modules = set() self.db_name = db_name - self.db = openerp.sql_db.db_connect(db_name) + self._db = openerp.sql_db.db_connect(db_name) + + # special cursor for test mode; None means "normal" mode + self.test_cr = None # Indicates that the registry is self.ready = False @@ -75,7 +78,7 @@ class Registry(Mapping): # Useful only in a multi-process context. self._any_cache_cleared = False - cr = self.db.cursor() + cr = self.cursor() has_unaccent = openerp.modules.db.has_unaccent(cr) if openerp.tools.config['unaccent'] and not has_unaccent: _logger.warning("The option --unaccent was given but no unaccent() function was found in database.") @@ -102,6 +105,10 @@ class Registry(Mapping): """ Return the model with the given name or raise KeyError if it doesn't exist.""" return self.models[model_name] + def __call__(self, model_name): + """ Same as ``self[model_name]``. """ + return self.models[model_name] + def do_parent_store(self, cr): for o in self._init_parent: self.get(o)._parent_store_compute(cr) @@ -183,27 +190,38 @@ class Registry(Mapping): r, c) return r, c - @contextmanager - def cursor(self, auto_commit=True): - cr = self.db.cursor() - try: - yield cr - if auto_commit: - cr.commit() - finally: - cr.close() + def enter_test_mode(self): + """ Enter the 'test' mode, where one cursor serves several requests. """ + assert self.test_cr is None + self.test_cr = self._db.test_cursor() + RegistryManager.enter_test_mode() -class TestRLock(object): - def __init__(self): - self._lock = threading.RLock() + def leave_test_mode(self): + """ Leave the test mode. """ + assert self.test_cr is not None + self.test_cr.close(force=True) # close the cursor for real + self.test_cr = None + RegistryManager.leave_test_mode() + + def cursor(self): + """ Return a new cursor for the database. The cursor itself may be used + as a context manager to commit/rollback and close automatically. + """ + if self.test_cr is not None: + # While in test mode, we use one special cursor across requests. The + # test cursor uses a reentrant lock to serialize accesses. The lock + # is granted here by cursor(), and automatically released by the + # cursor itself in its method close(). + self.test_cr.acquire() + return self.test_cr + return self._db.cursor() + +class DummyRLock(object): + """ Dummy reentrant lock, to be used while running rpc and js tests """ def acquire(self): - if openerp.tools.config['test_enable']: - return - return self._lock.acquire() + pass def release(self): - if openerp.tools.config['test_enable']: - return - return self._lock.release() + pass def __enter__(self): self.acquire() def __exit__(self, type, value, traceback): @@ -219,12 +237,30 @@ class RegistryManager(object): # Mapping between db name and model registry. # Accessed through the methods below. registries = {} - registries_lock = TestRLock() + _lock = threading.RLock() + _saved_lock = None + + @classmethod + def lock(cls): + """ Return the current registry lock. """ + return cls._lock + + @classmethod + def enter_test_mode(cls): + """ Enter the 'test' mode, where the registry is no longer locked. """ + assert cls._saved_lock is None + cls._lock, cls._saved_lock = DummyRLock(), cls._lock + + @classmethod + def leave_test_mode(cls): + """ Leave the 'test' mode. """ + assert cls._saved_lock is not None + cls._lock, cls._saved_lock = cls._saved_lock, None @classmethod def get(cls, db_name, force_demo=False, status=None, update_module=False): """ Return a registry for a given database name.""" - with cls.registries_lock: + with cls.lock(): try: return cls.registries[db_name] except KeyError: @@ -244,7 +280,7 @@ class RegistryManager(object): """ import openerp.modules - with cls.registries_lock: + with cls.lock(): registry = Registry(db_name) # Initializing a registry will call general code which will in turn @@ -259,7 +295,7 @@ class RegistryManager(object): registry.base_registry_signaling_sequence = seq_registry registry.base_cache_signaling_sequence = seq_cache # This should be a method on Registry - openerp.modules.load_modules(registry.db, force_demo, status, update_module) + openerp.modules.load_modules(registry._db, force_demo, status, update_module) except Exception: del cls.registries[db_name] raise @@ -269,7 +305,7 @@ class RegistryManager(object): # Yeah, crazy. registry = cls.registries[db_name] - cr = registry.db.cursor() + cr = registry.cursor() try: registry.do_parent_store(cr) cr.commit() @@ -286,7 +322,7 @@ class RegistryManager(object): @classmethod def delete(cls, db_name): """Delete the registry linked to a given database. """ - with cls.registries_lock: + with cls.lock(): if db_name in cls.registries: cls.registries[db_name].clear_caches() del cls.registries[db_name] @@ -294,7 +330,7 @@ class RegistryManager(object): @classmethod def delete_all(cls): """Delete all the registries. """ - with cls.registries_lock: + with cls.lock(): for db_name in cls.registries.keys(): cls.delete(db_name) @@ -309,7 +345,7 @@ class RegistryManager(object): This method is given to spare you a ``RegistryManager.get(db_name)`` that would loads the given database if it was not already loaded. """ - with cls.registries_lock: + with cls.lock(): if db_name in cls.registries: cls.registries[db_name].clear_caches() @@ -325,7 +361,7 @@ class RegistryManager(object): changed = False if openerp.multi_process and db_name in cls.registries: registry = cls.get(db_name) - cr = registry.db.cursor() + cr = registry.cursor() try: cr.execute(""" SELECT base_registry_signaling.last_value, @@ -371,7 +407,7 @@ class RegistryManager(object): registry = cls.get(db_name) if registry.any_cache_cleared(): _logger.info("At least one model cache has been cleared, signaling through the database.") - cr = registry.db.cursor() + cr = registry.cursor() r = 1 try: cr.execute("select nextval('base_cache_signaling')") @@ -386,7 +422,7 @@ class RegistryManager(object): if openerp.multi_process and db_name in cls.registries: _logger.info("Registry changed, signaling through the database") registry = cls.get(db_name) - cr = registry.db.cursor() + cr = registry.cursor() r = 1 try: cr.execute("select nextval('base_registry_signaling')") diff --git a/openerp/pooler.py b/openerp/pooler.py index 0df6f88c819..5ada4c2a014 100644 --- a/openerp/pooler.py +++ b/openerp/pooler.py @@ -36,7 +36,7 @@ def get_db_and_pool(db_name, force_demo=False, status=None, update_module=False) assert openerp.conf.deprecation.openerp_pooler _logger.warning('openerp.pooler.get_db_and_pool() is deprecated.') registry = RegistryManager.get(db_name, force_demo, status, update_module) - return registry.db, registry + return registry._db, registry def restart_pool(db_name, force_demo=False, status=None, update_module=False): @@ -44,7 +44,7 @@ def restart_pool(db_name, force_demo=False, status=None, update_module=False): _logger.warning('openerp.pooler.restart_pool() is deprecated.') assert openerp.conf.deprecation.openerp_pooler registry = RegistryManager.new(db_name, force_demo, status, update_module) - return registry.db, registry + return registry._db, registry def get_db(db_name): """Return a database connection. The corresponding registry is initialized.""" diff --git a/openerp/service/model.py b/openerp/service/model.py index a637e234295..a91baf8ca08 100644 --- a/openerp/service/model.py +++ b/openerp/service/model.py @@ -161,21 +161,10 @@ def execute_cr(cr, uid, obj, method, *args, **kw): def execute_kw(db, uid, obj, method, args, kw=None): return execute(db, uid, obj, method, *args, **kw or {}) -@contextmanager -def closing_cr_and_commit(cr): - try: - yield cr - cr.commit() - except Exception: - cr.rollback() - raise - finally: - cr.close() - @check def execute(db, uid, obj, method, *args, **kw): threading.currentThread().dbname = db - with closing_cr_and_commit(openerp.registry(db).db.cursor()) as cr: + with openerp.registry(db).cursor() as cr: if method.startswith('_'): raise except_orm('Access Denied', 'Private methods (such as %s) cannot be called remotely.' % (method,)) res = execute_cr(cr, uid, obj, method, *args, **kw) @@ -190,7 +179,7 @@ def exec_workflow_cr(cr, uid, obj, signal, *args): @check def exec_workflow(db, uid, obj, signal, *args): - with closing_cr_and_commit(openerp.registry(db).db.cursor()) as cr: + with openerp.registry(db).cursor() as cr: return exec_workflow_cr(cr, uid, obj, signal, *args) # vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4: diff --git a/openerp/service/report.py b/openerp/service/report.py index 370b11fc199..0c2f720f4cb 100644 --- a/openerp/service/report.py +++ b/openerp/service/report.py @@ -49,7 +49,7 @@ def exp_render_report(db, uid, object, ids, datas=None, context=None): self_reports[id] = {'uid': uid, 'result': False, 'state': False, 'exception': None} - cr = openerp.registry(db).db.cursor() + cr = openerp.registry(db).cursor() try: result, format = openerp.report.render_report(cr, uid, ids, object, datas, context) if not result: @@ -87,7 +87,7 @@ def exp_report(db, uid, object, ids, datas=None, context=None): self_reports[id] = {'uid': uid, 'result': False, 'state': False, 'exception': None} def go(id, uid, ids, datas, context): - cr = openerp.registry(db).db.cursor() + cr = openerp.registry(db).cursor() try: result, format = openerp.report.render_report(cr, uid, ids, object, datas, context) if not result: diff --git a/openerp/sql_db.py b/openerp/sql_db.py index 59841656d8c..ee58503c40b 100644 --- a/openerp/sql_db.py +++ b/openerp/sql_db.py @@ -347,6 +347,23 @@ class Cursor(object): """ return self._cnx.rollback() + def __enter__(self): + """ Using the cursor as a contextmanager automatically commits and + closes it:: + + with cr: + cr.execute(...) + + # cr is committed if no failure occurred + # cr is closed in any case + """ + return self + + def __exit__(self, exc_type, exc_value, traceback): + if exc_type is None: + self.commit() + self.close() + @contextmanager @check def savepoint(self): @@ -364,6 +381,42 @@ class Cursor(object): def __getattr__(self, name): return getattr(self._obj, name) +class TestCursor(Cursor): + """ A cursor to be used for tests. It keeps the transaction open across + several requests, and simulates committing, rolling back, and closing. + """ + def __init__(self, *args, **kwargs): + super(TestCursor, self).__init__(*args, **kwargs) + # in order to simulate commit and rollback, the cursor maintains a + # savepoint at its last commit + self.execute("SAVEPOINT test_cursor") + # we use a lock to serialize concurrent requests + self._lock = threading.RLock() + + def acquire(self): + self._lock.acquire() + + def release(self): + self._lock.release() + + def close(self, force=False): + if force: + super(TestCursor, self).close() + elif not self._closed: + self.rollback() # for stuff that has not been committed + self.release() + + def autocommit(self, on): + _logger.debug("TestCursor.autocommit(%r) does nothing", on) + + def commit(self): + self.execute("RELEASE SAVEPOINT test_cursor") + self.execute("SAVEPOINT test_cursor") + + def rollback(self): + self.execute("ROLLBACK TO SAVEPOINT test_cursor") + self.execute("SAVEPOINT test_cursor") + class PsycoConnection(psycopg2.extensions.connection): pass @@ -491,6 +544,11 @@ class Connection(object): _logger.debug('create %scursor to %r', cursor_type, self.dbname) return Cursor(self._pool, self.dbname, serialized=serialized) + def test_cursor(self, serialized=True): + cursor_type = serialized and 'serialized ' or '' + _logger.debug('create test %scursor to %r', cursor_type, self.dbname) + return TestCursor(self._pool, self.dbname, serialized=serialized) + # serialized_cursor is deprecated - cursors are serialized by default serialized_cursor = cursor diff --git a/openerp/tests/common.py b/openerp/tests/common.py index c1f7cf88ed1..9f3684fa6df 100644 --- a/openerp/tests/common.py +++ b/openerp/tests/common.py @@ -20,6 +20,7 @@ from datetime import datetime, timedelta import werkzeug import openerp +from openerp.modules.registry import RegistryManager _logger = logging.getLogger(__name__) @@ -37,25 +38,6 @@ if not DB and hasattr(threading.current_thread(), 'dbname'): # Useless constant, tests are aware of the content of demo data ADMIN_USER_ID = openerp.SUPERUSER_ID -# Magic session_id, unfortunately we have to serialize access to the cursors to -# serialize requests. We first tried to duplicate the database for each tests -# but this proved too slow. Any idea to improve this is welcome. -HTTP_SESSION = {} - -def acquire_test_cursor(session_id): - if openerp.tools.config['test_enable']: - cr = HTTP_SESSION.get(session_id) - if cr: - cr._test_lock.acquire() - return cr - -def release_test_cursor(cr): - if openerp.tools.config['test_enable']: - if hasattr(cr, '_test_lock'): - cr._test_lock.release() - return True - return False - def at_install(flag): """ Sets the at-install state of a test, the flag is a boolean specifying whether the test should (``True``) or should not (``False``) run during @@ -67,6 +49,7 @@ def at_install(flag): obj.at_install = flag return obj return decorator + def post_install(flag): """ Sets the post-install state of a test. The flag is a boolean specifying whether the test should or should not run after a set of @@ -83,18 +66,13 @@ class BaseCase(unittest2.TestCase): """ Subclass of TestCase for common OpenERP-specific code. - This class is abstract and expects self.cr and self.uid to be initialized by subclasses. + This class is abstract and expects self.registry, self.cr and self.uid to be + initialized by subclasses. """ - @classmethod def cursor(self): - return openerp.modules.registry.RegistryManager.get(DB).db.cursor() + return self.registry.cursor() - @classmethod - def registry(self, model): - return openerp.modules.registry.RegistryManager.get(DB)[model] - - @classmethod def ref(self, xid): """ Returns database ID corresponding to a given identifier. @@ -106,7 +84,6 @@ class BaseCase(unittest2.TestCase): _, id = self.registry('ir.model.data').get_object_reference(self.cr, self.uid, module, xid) return id - @classmethod def browse_ref(self, xid): """ Returns a browsable record for the given identifier. @@ -125,10 +102,9 @@ class TransactionCase(BaseCase): """ def setUp(self): - # Store cr and uid in class variables, to allow ref() and browse_ref to be BaseCase @classmethods - # and still access them - TransactionCase.cr = self.cursor() - TransactionCase.uid = openerp.SUPERUSER_ID + self.registry = RegistryManager.get(DB) + self.cr = self.cursor() + self.uid = openerp.SUPERUSER_ID def tearDown(self): self.cr.rollback() @@ -143,7 +119,8 @@ class SingleTransactionCase(BaseCase): @classmethod def setUpClass(cls): - cls.cr = cls.cursor() + cls.registry = RegistryManager.get(DB) + cls.cr = cls.registry.cursor() cls.uid = openerp.SUPERUSER_ID @classmethod @@ -166,16 +143,15 @@ class HttpCase(TransactionCase): def setUp(self): super(HttpCase, self).setUp() + self.registry.enter_test_mode() # setup a magic session_id that will be rollbacked self.session = openerp.http.root.session_store.new() self.session_id = self.session.sid self.session.db = DB openerp.http.root.session_store.save(self.session) - self.cr._test_lock = threading.RLock() - HTTP_SESSION[self.session_id] = self.cr def tearDown(self): - del HTTP_SESSION[self.session_id] + self.registry.leave_test_mode() super(HttpCase, self).tearDown() def url_open(self, url, data=None, timeout=10): diff --git a/openerp/tools/mail.py b/openerp/tools/mail.py index 85dcfd368fe..de6cd173521 100644 --- a/openerp/tools/mail.py +++ b/openerp/tools/mail.py @@ -625,7 +625,7 @@ def email_send(email_from, email_to, subject, body, email_cc=None, email_bcc=Non if not cr: db_name = getattr(threading.currentThread(), 'dbname', None) if db_name: - local_cr = cr = openerp.registry(db_name).db.cursor() + local_cr = cr = openerp.registry(db_name).cursor() else: raise Exception("No database cursor found, please pass one explicitly") diff --git a/openerpcommand/module.py b/openerpcommand/module.py index 550e0a2be28..284c0186785 100644 --- a/openerpcommand/module.py +++ b/openerpcommand/module.py @@ -33,12 +33,9 @@ def run(args): xs = [] ir_module_module = registry.get('ir.module.module') - cr = registry.db.cursor() # TODO context manager - try: + with registry.cursor() as cr: ids = ir_module_module.search(cr, openerp.SUPERUSER_ID, [], {}) xs = ir_module_module.read(cr, openerp.SUPERUSER_ID, ids, [], {}) - finally: - cr.close() if xs: print "Modules (database `%s`):" % (args.database,) diff --git a/openerpcommand/read.py b/openerpcommand/read.py index 5b68fb26ef0..61c05117875 100644 --- a/openerpcommand/read.py +++ b/openerpcommand/read.py @@ -20,15 +20,12 @@ def run(args): registry = openerp.modules.registry.RegistryManager.get( args.database, update_module=False) model = registry[args.model] - cr = registry.db.cursor() # TODO context manager field_names = [args.field] if args.field else [] if args.short: # ignore --field field_names = ['name'] - try: + with registry.cursor() as cr: xs = model.read(cr, 1, args.id, field_names, {}) - finally: - cr.close() if xs: print "Records (model `%s`, database `%s`):" % (args.model, args.database) diff --git a/openerpcommand/uninstall.py b/openerpcommand/uninstall.py index 27e4e04002a..244b0575e53 100644 --- a/openerpcommand/uninstall.py +++ b/openerpcommand/uninstall.py @@ -43,15 +43,12 @@ def run(args): args.database, update_module=False) ir_module_module = registry.get('ir.module.module') - cr = registry.db.cursor() # TODO context manager - try: + with registry.cursor() as cr: ids = ir_module_module.search(cr, openerp.SUPERUSER_ID, [('name', 'in', args.module), ('state', '=', 'installed')], {}) if len(ids) == len(args.module): ir_module_module.button_immediate_uninstall(cr, openerp.SUPERUSER_ID, ids, {}) else: print "At least one module not found (database `%s`)." % (args.database,) - finally: - cr.close() def add_parser(subparsers): parser = subparsers.add_parser('uninstall',