From 6bc60505f4e25d5ce7181f30d33692a8e7d08fe0 Mon Sep 17 00:00:00 2001
From: Raphael Collet <rco@openerp.com>
Date: Tue, 8 Apr 2014 14:51:22 +0200
Subject: [PATCH] [IMP] registry: add method get_cursor() to simply retrieve a
 new cursor, and refactor code to use it

bzr revid: rco@openerp.com-20140408125122-ki0zmin3m21k2itd
---
 openerp/addons/base/res/res_users.py          |  6 +++---
 openerp/addons/base/tests/test_ir_sequence.py |  2 +-
 openerp/addons/base/tests/test_uninstall.py   |  2 +-
 openerp/cli/server.py                         |  4 ++--
 openerp/http.py                               |  2 +-
 openerp/modules/registry.py                   | 17 +++++++++++------
 openerp/service/model.py                      | 15 ++-------------
 openerp/service/report.py                     |  4 ++--
 openerp/tests/common.py                       |  4 ++--
 openerp/tools/mail.py                         |  2 +-
 openerpcommand/module.py                      |  5 +----
 openerpcommand/read.py                        |  5 +----
 openerpcommand/uninstall.py                   |  5 +----
 13 files changed, 29 insertions(+), 44 deletions(-)

diff --git a/openerp/addons/base/res/res_users.py b/openerp/addons/base/res/res_users.py
index 772f7f7290b..0ad1d1276f6 100644
--- a/openerp/addons/base/res/res_users.py
+++ b/openerp/addons/base/res/res_users.py
@@ -388,7 +388,7 @@ class res_users(osv.osv):
         if not password:
             return False
         user_id = False
-        cr = self.pool.db.cursor()
+        cr = self.pool.get_cursor()
         try:
             # autocommit: our single update request will be performed atomically.
             # (In this way, there is no opportunity to have two transactions
@@ -440,7 +440,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.get_cursor()
                 try:
                     base = user_agent_env['base_location']
                     ICP = self.pool['ir.config_parameter']
@@ -461,7 +461,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.get_cursor()
         try:
             self.check_credentials(cr, uid, passwd)
             if self._uid_cache.has_key(db):
diff --git a/openerp/addons/base/tests/test_ir_sequence.py b/openerp/addons/base/tests/test_ir_sequence.py
index e925fffa3b1..5bb64720336 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).get_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..7c88196fbf6 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).get_cursor()
 
 def get_module(module_name):
     registry = openerp.modules.registry.RegistryManager.get(DB)
diff --git a/openerp/cli/server.py b/openerp/cli/server.py
index e7ba695f19c..a808511ab8a 100644
--- a/openerp/cli/server.py
+++ b/openerp/cli/server.py
@@ -103,7 +103,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.get_cursor()
     openerp.tools.trans_export(config["language"],
         config["translate_modules"] or ["all"], buf, fileformat, cr)
     cr.close()
@@ -117,7 +117,7 @@ def import_translation():
     dbname = config['db_name']
 
     registry = openerp.modules.registry.RegistryManager.new(dbname)
-    cr = registry.db.cursor()
+    cr = registry.get_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 5f37afbc116..d346211c89b 100644
--- a/openerp/http.py
+++ b/openerp/http.py
@@ -238,7 +238,7 @@ class WebRequest(object):
             # 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.get_cursor()
         return self._cr
 
     def __enter__(self):
diff --git a/openerp/modules/registry.py b/openerp/modules/registry.py
index 2db14744915..60f56ef035c 100644
--- a/openerp/modules/registry.py
+++ b/openerp/modules/registry.py
@@ -75,7 +75,7 @@ class Registry(Mapping):
         # Useful only in a multi-process context.
         self._any_cache_cleared = False
 
-        cr = self.db.cursor()
+        cr = self.get_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.")
@@ -187,9 +187,14 @@ class Registry(Mapping):
                     r, c)
         return r, c
 
+    def get_cursor(self):
+        """ Return a new cursor for the database. """
+        return self.db.cursor()
+
     @contextmanager
     def cursor(self, auto_commit=True):
-        cr = self.db.cursor()
+        """ Manage a new cursor; commit, rollback and closing are automatic. """
+        cr = self.get_cursor()
         try:
             yield cr
             if auto_commit:
@@ -286,7 +291,7 @@ class RegistryManager(object):
             # Yeah, crazy.
             registry = cls.registries[db_name]
 
-            cr = registry.db.cursor()
+            cr = registry.get_cursor()
             try:
                 registry.do_parent_store(cr)
                 cr.commit()
@@ -342,7 +347,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.get_cursor()
             try:
                 cr.execute("""
                     SELECT base_registry_signaling.last_value,
@@ -388,7 +393,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.get_cursor()
                 r = 1
                 try:
                     cr.execute("select nextval('base_cache_signaling')")
@@ -403,7 +408,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.get_cursor()
             r = 1
             try:
                 cr.execute("select nextval('base_registry_signaling')")
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..30e043c2977 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).get_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).get_cursor()
         try:
             result, format = openerp.report.render_report(cr, uid, ids, object, datas, context)
             if not result:
diff --git a/openerp/tests/common.py b/openerp/tests/common.py
index 6c4864e46d5..4b20c0bc7c4 100644
--- a/openerp/tests/common.py
+++ b/openerp/tests/common.py
@@ -89,7 +89,7 @@ class BaseCase(unittest2.TestCase):
     """
 
     def cursor(self):
-        return self.registry.db.cursor()
+        return self.registry.get_cursor()
 
     def ref(self, xid):
         """ Returns database ID corresponding to a given identifier.
@@ -138,7 +138,7 @@ class SingleTransactionCase(BaseCase):
     @classmethod
     def setUpClass(cls):
         cls.registry = openerp.modules.registry.RegistryManager.get(DB)
-        cls.cr = cls.registry.db.cursor()
+        cls.cr = cls.registry.get_cursor()
         cls.uid = openerp.SUPERUSER_ID
 
     @classmethod
diff --git a/openerp/tools/mail.py b/openerp/tools/mail.py
index 85dcfd368fe..3da075fda6e 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).get_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',