Merge pull request #51 from NightJar/tighten-transactions

FIX Tighten transactions

code/SQLite3Database.php

@@ -65,6 +65,11 @@ class SQLite3Database extends Database
      */
     protected $transactionNesting = 0;
 
+    /**
+     * @var array
+     */
+    protected $transactionSavepoints = [];
+
     /**
      * List of default pragma values
      *
@@ -470,17 +475,28 @@ class SQLite3Database extends Database
 
     public function transactionStart($transaction_mode = false, $session_characteristics = false)
     {
-        if ($this->transactionNesting > 0) {
+        if ($this->transactionDepth()) {
-            $this->transactionSavepoint('NESTEDTRANSACTION' . $this->transactionNesting);
+            $this->transactionSavepoint('NESTEDTRANSACTION' . $this->transactionDepth());
         } else {
             $this->query('BEGIN');
+            $this->transactionDepthIncrease();
         }
-        ++$this->transactionNesting;
     }
 
     public function transactionSavepoint($savepoint)
     {
         $this->query("SAVEPOINT \"$savepoint\"");
+        $this->transactionDepthIncrease($savepoint);
+    }
+
+    /**
+     * Fetch the name of the most recent savepoint
+     *
+     * @return string
+     */
+    protected function getTransactionSavepointName()
+    {
+        return end($this->transactionSavepoints);
     }
 
     public function transactionRollback($savepoint = false)
@@ -488,19 +504,20 @@ class SQLite3Database extends Database
         // Named transaction
         if ($savepoint) {
             $this->query("ROLLBACK TO $savepoint;");
+            $this->transactionDepthDecrease();
             return true;
         }
 
         // Fail if transaction isn't available
-        if (!$this->transactionNesting) {
+        if (!$this->transactionDepth()) {
             return false;
         }
 
-        --$this->transactionNesting;
+        if ($this->transactionIsNested()) {
-        if ($this->transactionNesting > 0) {
+            $this->transactionRollback($this->getTransactionSavepointName());
-            $this->transactionRollback('NESTEDTRANSACTION' . $this->transactionNesting);
         } else {
             $this->query('ROLLBACK;');
+            $this->transactionDepthDecrease();
         }
         return true;
     }
@@ -513,49 +530,93 @@ class SQLite3Database extends Database
     public function transactionEnd($chain = false)
     {
         // Fail if transaction isn't available
-        if (!$this->transactionNesting) {
+        if (!$this->transactionDepth()) {
             return false;
         }
-        --$this->transactionNesting;
+
-        if ($this->transactionNesting <= 0) {
+        if ($this->transactionIsNested()) {
-            $this->transactionNesting = 0;
+            $savepoint = $this->getTransactionSavepointName();
+            $this->query('RELEASE ' . $savepoint);
+            $this->transactionDepthDecrease();
+        } else {
             $this->query('COMMIT;');
+            $this->resetTransactionNesting();
         }
+
+        if ($chain) {
+            $this->transactionStart();
+        }
+
         return true;
     }
 
+    /**
+     * Indicate whether or not the current transaction is nested
+     * Returns false if there are no transactions, or the open
+     * transaction is the 'outer' transaction, i.e. not nested.
+     *
+     * @return bool
+     */
+    protected function transactionIsNested()
+    {
+        return $this->transactionNesting > 1;
+    }
+
+    /**
+     * Increase the nested transaction level by one
+     * savepoint tracking is optional because BEGIN
+     * opens a transaction, but is not a named reference
+     *
+     * @param string $savepoint
+     */
+    protected function transactionDepthIncrease($savepoint = null)
+    {
+        ++$this->transactionNesting;
+        if ($savepoint) {
+            array_push($this->transactionSavepoints, $savepoint);
+        }
+    }
+
+    /**
+     * Decrease the nested transaction level by one
+     * and reduce the savepoint tracking if we are
+     * nesting, as the last one is no longer valid
+     */
+    protected function transactionDepthDecrease()
+    {
+        if ($this->transactionIsNested()) {
+            array_pop($this->transactionSavepoints);
+        }
+        --$this->transactionNesting;
+    }
+
     /**
      * In error condition, set transactionNesting to zero
      */
     protected function resetTransactionNesting()
     {
         $this->transactionNesting = 0;
+        $this->transactionSavepoints = [];
     }
 
     public function query($sql, $errorLevel = E_USER_ERROR)
     {
-        $this->inspectQuery($sql);
         return parent::query($sql, $errorLevel);
     }
 
     public function preparedQuery($sql, $parameters, $errorLevel = E_USER_ERROR)
     {
-        $this->inspectQuery($sql);
         return parent::preparedQuery($sql, $parameters, $errorLevel);
     }
 
     /**
      * Inspect a SQL query prior to execution
-     *
+     * @deprecated 2.2.0:3.0.0
      * @param string $sql
      */
     protected function inspectQuery($sql)
     {
-        // Any DDL discards transactions.
+        // no-op
-        $isDDL = $this->getConnector()->isQueryDDL($sql);
-        if ($isDDL) {
-            $this->resetTransactionNesting();
-        }
     }
 
     public function clearTable($table)

code/SQLite3SchemaManager.php

@@ -276,21 +276,22 @@ class SQLite3SchemaManager extends DBSchemaManager
         }
 
         $queries = array(
-            "BEGIN TRANSACTION",
             "CREATE TABLE \"{$tableName}_alterfield_{$fieldName}\"(" . implode(',', $newColsSpec) . ")",
             "INSERT INTO \"{$tableName}_alterfield_{$fieldName}\" SELECT {$fieldNameList} FROM \"$tableName\"",
             "DROP TABLE \"$tableName\"",
             "ALTER TABLE \"{$tableName}_alterfield_{$fieldName}\" RENAME TO \"$tableName\"",
-            "COMMIT"
         );
 
         // Remember original indexes
         $indexList = $this->indexList($tableName);
 
         // Then alter the table column
+        $database = $this->database;
+        $database->withTransaction(function () use ($database, $queries, $indexList) {
             foreach ($queries as $query) {
-            $this->query($query.';');
+                $database->query($query . ';');
             }
+        });
 
         // Recreate the indexes
         foreach ($indexList as $indexName => $indexSpec) {
@@ -319,21 +320,22 @@ class SQLite3SchemaManager extends DBSchemaManager
         $oldColsStr = implode(',', $oldCols);
         $newColsSpecStr = implode(',', $newColsSpec);
         $queries = array(
-            "BEGIN TRANSACTION",
             "CREATE TABLE \"{$tableName}_renamefield_{$oldName}\" ({$newColsSpecStr})",
             "INSERT INTO \"{$tableName}_renamefield_{$oldName}\" SELECT {$oldColsStr} FROM \"$tableName\"",
             "DROP TABLE \"$tableName\"",
             "ALTER TABLE \"{$tableName}_renamefield_{$oldName}\" RENAME TO \"$tableName\"",
-            "COMMIT"
         );
 
         // Remember original indexes
         $oldIndexList = $this->indexList($tableName);
 
         // Then alter the table column
+        $database = $this->database;
+        $database->withTransaction(function () use ($database, $queries) {
             foreach ($queries as $query) {
-            $this->query($query.';');
+                $database->query($query . ';');
             }
+        });
 
         // Recreate the indexes
         foreach ($oldIndexList as $indexName => $indexSpec) {
@@ -430,6 +432,15 @@ class SQLite3SchemaManager extends DBSchemaManager
         return $this->buildSQLiteIndexName($table, $index);
     }
 
+    protected function convertIndexSpec($indexSpec)
+    {
+        $supportedIndexTypes = ['index', 'unique'];
+        if (isset($indexSpec['type']) && !in_array($indexSpec['type'], $supportedIndexTypes)) {
+            $indexSpec['type'] = 'index';
+        }
+        return parent::convertIndexSpec($indexSpec);
+    }
+
     public function indexList($table)
     {
         $indexList = array();