Refactore Transaction to add close and autocommit

review2151002

doc/ref/transaction.rst

@@ -17,6 +17,10 @@ database transaction.
 
     The database cursor.
 
+.. attribute:: Transaction.database
+
+    The database.
+
 .. attribute:: Transaction.user
 
     The id of the user.
@@ -39,7 +43,7 @@ database transaction.
 
     Count the number of modification made in this transaction.
 
-.. method:: Transaction.start(database_name, user[, context])
+.. method:: Transaction.start(database_name, user[, readonly[, context[, close[, autocommit]]]])
 
     Start a new transaction and return a `context manager`_.
 
@@ -64,7 +68,7 @@ database transaction.
     Modify the cursor of the transaction and return a `context manager`_. The
     previous cursor will be restored when exiting the `with` statement.
 
-.. method:: Transaction.new_cursor()
+.. method:: Transaction.new_cursor([autocommit[, readonly]])
 
     Change the cursor of the transaction with a new one on the same database
     and return a `context manager`_. The previous cursor will be restored when

trytond/backend/database.py

@@ -41,7 +41,8 @@ class DatabaseInterface(object):
         '''
         raise NotImplementedError
 
-    def create(self, cursor, database_name):
+    @staticmethod
+    def create(cursor, database_name):
         '''
         Create a database
 
@@ -49,7 +50,8 @@ class DatabaseInterface(object):
         '''
         raise NotImplementedError
 
-    def drop(self, cursor, database_name):
+    @staticmethod
+    def drop(cursor, database_name):
         '''
         Drop a database
 

trytond/backend/mysql/database.py

@@ -105,14 +105,16 @@ class Database(DatabaseInterface):
     def close(self):
         return
 
-    def create(self, cursor, database_name):
+    @classmethod
+    def create(cls, cursor, database_name):
         cursor.execute('CREATE DATABASE `' + database_name + '` '
             'DEFAULT CHARACTER SET = \'utf8\'')
-        Database._list_cache = None
+        cls._list_cache = None
 
-    def drop(self, cursor, database_name):
+    @classmethod
+    def drop(cls, cursor, database_name):
         cursor.execute('DROP DATABASE `' + database_name + '`')
-        Database._list_cache = None
+        cls._list_cache = None
 
     @staticmethod
     def dump(database_name):

trytond/backend/postgresql/database.py

@@ -85,14 +85,16 @@ class Database(DatabaseInterface):
         self._connpool.closeall()
         self._connpool = None
 
-    def create(self, cursor, database_name):
+    @classmethod
+    def create(cls, cursor, database_name):
         cursor.execute('CREATE DATABASE "' + database_name + '" '
             'TEMPLATE template0 ENCODING \'unicode\'')
-        Database._list_cache = None
+        cls._list_cache = None
 
-    def drop(self, cursor, database_name):
+    @classmethod
+    def drop(cls, cursor, database_name):
         cursor.execute('DROP DATABASE "' + database_name + '"')
-        Database._list_cache = None
+        cls._list_cache = None
 
     def get_version(self, cursor):
         if self.database_name not in self._version_cache:

trytond/backend/sqlite/database.py

@@ -148,8 +148,7 @@ class Database(DatabaseInterface):
 
     def __new__(cls, database_name=':memory:'):
         if (database_name == ':memory:'
-                and hasattr(cls._local, 'memory_database')
-                and cls._local.memory_database):
+                and getattr(cls._local, 'memory_database', None)):
             return cls._local.memory_database
         return DatabaseInterface.__new__(cls, database_name=database_name)
 
@@ -197,7 +196,8 @@ class Database(DatabaseInterface):
             return
         self._conn = None
 
-    def create(self, cursor, database_name):
+    @staticmethod
+    def create(cursor, database_name):
         if database_name == ':memory:':
             path = ':memory:'
         else:
@@ -209,9 +209,10 @@ class Database(DatabaseInterface):
             cursor = conn.cursor()
             cursor.close()
 
-    def drop(self, cursor, database_name):
+    @classmethod
+    def drop(cls, cursor, database_name):
         if database_name == ':memory:':
-            self._conn = None
+            cls._local.memory_database._conn = None
             return
         if os.sep in database_name:
             return

trytond/protocols/dispatcher.py

@@ -83,17 +83,8 @@ def dispatch(host, port, protocol, database_name, user, session, object_type,
         elif method == 'list':
             if CONFIG['prevent_dblist']:
                 raise Exception('AccessDenied')
-            database = Database().connect()
-            Flavor.set(Database.flavor)
-            try:
-                cursor = database.cursor()
-                try:
-                    res = database.list(cursor)
-                finally:
-                    cursor.close(close=True)
-            except Exception:
-                res = []
-            return res
+            with Transaction().start(None, 0, close=True) as transaction:
+                return transaction.database.list(transaction.cursor)
         elif method == 'create':
             return create(*args, **kwargs)
         elif method == 'restore':
@@ -222,18 +213,13 @@ def create(database_name, password, lang, admin_password):
     logger = logging.getLogger('database')
 
     try:
-        database = Database().connect()
-        cursor = database.cursor(autocommit=True)
-        try:
-            database.create(cursor, database_name)
-            cursor.commit()
-            cursor.close(close=True)
-        except Exception:
-            cursor.close()
-            raise
+        with Transaction().start(None, 0, close=True, autocommit=True) \
+            as transaction:
+            transaction.database.create(transaction.cursor, database_name)
+            transaction.cursor.commit()
 
         with Transaction().start(database_name, 0) as transaction:
-            database.init(transaction.cursor)
+            Database.init(transaction.cursor)
             transaction.cursor.execute(*ir_configuration.insert(
                     [ir_configuration.language], [[lang]]))
             transaction.cursor.commit()
@@ -277,11 +263,11 @@ def drop(database_name, password):
     time.sleep(1)
     logger = logging.getLogger('database')
 
-    database = Database().connect()
-    cursor = database.cursor(autocommit=True)
-    try:
+    with Transaction().start(None, 0, close=True, autocommit=True) \
+        as transaction:
+        cursor = transaction.cursor
         try:
-            database.drop(cursor, database_name)
+            Database.drop(cursor, database_name)
             cursor.commit()
         except Exception:
             logger.error('DROP DB: %s failed' % (database_name,))
@@ -291,8 +277,6 @@ def drop(database_name, password):
         else:
             logger.info('DROP DB: %s' % (database_name))
             Pool.stop(database_name)
-    finally:
-        cursor.close(close=True)
     return True
 
 

trytond/tests/test_tryton.py

@@ -217,6 +217,18 @@ def test_depends():
                         list(depends - set(model._fields)), mname, fname))
 
 
+def doctest_dropdb(test):
+    '''Remove SQLite memory database'''
+    from trytond.backend.sqlite.database import Database as SQLiteDatabase
+    database = SQLiteDatabase().connect()
+    cursor = database.cursor(autocommit=True)
+    try:
+        SQLiteDatabase.drop(cursor, ':memory:')
+        cursor.commit()
+    finally:
+        cursor.close()
+
+
 def suite():
     '''
     Return test suite for other modules

trytond/transaction.py

@@ -58,6 +58,8 @@ class Transaction(local):
     __metaclass__ = Singleton
 
     cursor = None
+    database = None
+    close = None
     user = None
     context = None
     create_records = None
@@ -65,20 +67,24 @@ class Transaction(local):
     delete = None  # TODO check to merge with delete_records
     timestamp = None
 
-    def start(self, database_name, user, readonly=False, context=None):
+    def start(self, database_name, user, readonly=False, context=None,
+            close=False, autocommit=False):
         '''
         Start transaction
         '''
         Database = backend.get('Database')
         assert self.user is None
+        assert self.database is None
         assert self.cursor is None
+        assert self.close is None
         assert self.context is None
         if not database_name:
-            database = Database().connect()
+            self.database = Database().connect()
         else:
-            database = Database(database_name).connect()
+            self.database = Database(database_name).connect()
         Flavor.set(Database.flavor)
-        self.cursor = database.cursor(readonly=readonly)
+        self.cursor = self.database.cursor(readonly=readonly,
+            autocommit=autocommit)
         self.user = user
         self.context = context or {}
         self.create_records = {}
@@ -93,9 +99,11 @@ class Transaction(local):
         Stop transaction
         '''
         try:
-            self.cursor.close()
+            self.cursor.close(close=self.close)
         finally:
             self.cursor = None
+            self.database = None
+            self.close = None
             self.user = None
             self.context = None
             self.create_records = None
@@ -136,11 +144,11 @@ class Transaction(local):
         self.cursor = cursor
         return manager
 
-    def new_cursor(self):
+    def new_cursor(self, autocommit=False, readonly=False):
         Database = backend.get('Database')
         manager = _CursorManager(self.cursor)
         database = Database(self.cursor.database_name).connect()
-        self.cursor = database.cursor()
+        self.cursor = database.cursor(autocommit=autocommit, readonly=readonly)
         return manager
 
     @property