Refactoring database transaction handling, fixing Model test for SQLite, all tests now pass

git-svn-id: https://svn.cakephp.org/repo/branches/1.2.x.x@6931 3807eeeb-6ff5-0310-8944-8be069107fe0

cake/libs/model/datasources/datasource.php

@@ -249,15 +249,15 @@ class DataSource extends Object {
 /**
  * Begin a transaction
  *
- * @return boolean True
+ * @return boolean Returns true if a transaction is not in progress
  */
-	function begin() {
+	function begin(&$model) {
 		return !$this->_transactionStarted;
 	}
 /**
  * Commit a transaction
  *
- * @return boolean True
+ * @return boolean Returns true if a transaction is in progress
  */
 	function commit(&$model) {
 		return $this->_transactionStarted;
@@ -265,7 +265,7 @@ class DataSource extends Object {
 /**
  * Rollback a transaction
  *
- * @return boolean True
+ * @return boolean Returns true if a transaction is in progress
  */
 	function rollback(&$model) {
 		return $this->_transactionStarted;

cake/libs/model/datasources/dbo/dbo_mysql.php

@@ -54,6 +54,17 @@ class DboMysql extends DboSource {
  * @var unknown_type
  */
 	var $endQuote = "`";
+/**
+ * Index of basic SQL commands
+ *
+ * @var array
+ * @access protected
+ */
+	var $_commands = array(
+		'begin'    => 'START TRANSACTION',
+		'commit'   => 'COMMIT',
+		'rollback' => 'ROLLBACK'
+	);
 /**
  * Base configuration settings for MySQL driver
  *
@@ -288,51 +299,6 @@ class DboMysql extends DboSource {
 		}
 		return true;
 	}
-/**
- * Begin a transaction
- *
- * @param unknown_type $model
- * @return boolean True on success, false on fail
- * (i.e. if the database/model does not support transactions).
- */
-	function begin(&$model) {
-		if (parent::begin($model)) {
-			if ($this->execute('START TRANSACTION')) {
-				$this->_transactionStarted = true;
-				return true;
-			}
-		}
-		return false;
-	}
-/**
- * Commit a transaction
- *
- * @param unknown_type $model
- * @return boolean True on success, false on fail
- * (i.e. if the database/model does not support transactions,
- * or a transaction has not started).
- */
-	function commit(&$model) {
-		if (parent::commit($model)) {
-			$this->_transactionStarted = false;
-			return $this->execute('COMMIT');
-		}
-		return false;
-	}
-/**
- * Rollback a transaction
- *
- * @param unknown_type $model
- * @return boolean True on success, false on fail
- * (i.e. if the database/model does not support transactions,
- * or a transaction has not started).
- */
-	function rollback(&$model) {
-		if (parent::rollback($model)) {
-			return $this->execute('ROLLBACK');
-		}
-		return false;
-	}
 /**
  * Returns a formatted error message from previous database operation.
  *

cake/libs/model/datasources/dbo/dbo_postgres.php

@@ -38,8 +38,30 @@
  */
 class DboPostgres extends DboSource {
 
+/**
+ * Driver description
+ *
+ * @var string
+ * @access public
+ */
 	var $description = "PostgreSQL DBO Driver";
-
+/**
+ * Index of basic SQL commands
+ *
+ * @var array
+ * @access protected
+ */
+	var $_commands = array(
+		'begin'    => 'BEGIN',
+		'commit'   => 'COMMIT',
+		'rollback' => 'ROLLBACK'
+	);
+/**
+ * Base driver configuration settings.  Merged with user settings.
+ *
+ * @var array
+ * @access protected
+ */
 	var $_baseConfig = array(
 		'connect'	=> 'pg_pconnect',
 		'persistent' => true,
@@ -255,21 +277,6 @@ class DboPostgres extends DboSource {
 		}
 		return "'" . $data . "'";
 	}
-
-/**
- * Begin a transaction
- *
- * @param unknown_type $model
- * @return boolean True on success, false on fail
- * (i.e. if the database/model does not support transactions).
- */
-	function begin(&$model) {
-		if (parent::begin($model) && $this->execute('BEGIN')) {
-			$this->_transactionStarted = true;
-			return true;
-		}
-		return false;
-	}
 /**
  * Returns a formatted error message from previous database operation.
  *
@@ -602,8 +609,8 @@ class DboPostgres extends DboSource {
  */
 	function buildIndex($indexes, $table = null) {
 		$join = array();
-		foreach ($indexes as $name => $value) {
 
+		foreach ($indexes as $name => $value) {
 			if ($name == 'PRIMARY') {
 				$out = 'PRIMARY KEY  (' . $this->name($value['column']) . ')';
 			} else {

cake/libs/model/datasources/dbo/dbo_sqlite.php

@@ -44,17 +44,24 @@ class DboSqlite extends DboSource {
  */
 	var $description = "SQLite DBO Driver";
 /**
- * Enter description here...
+ * Opening quote for quoted identifiers
  *
- * @var unknown_type
+ * @var string
  */
 	var $startQuote = '"';
 /**
- * Enter description here...
+ * Closing quote for quoted identifiers
  *
- * @var unknown_type
+ * @var string
  */
 	var $endQuote = '"';
+/**
+ * Keeps the transaction statistics of CREATE/UPDATE/DELETE queries
+ *
+ * @var array
+ * @access protected
+ */
+	var $_queryStats = array();
 /**
  * Base configuration settings for SQLite driver
  *
@@ -65,6 +72,17 @@ class DboSqlite extends DboSource {
 		'database' => null,
 		'connect' => 'sqlite_popen'
 	);
+/**
+ * Index of basic SQL commands
+ *
+ * @var array
+ * @access protected
+ */
+	var $_commands = array(
+		'begin'    => 'BEGIN TRANSACTION',
+		'commit'   => 'COMMIT TRANSACTION',
+		'rollback' => 'ROLLBACK TRANSACTION'
+	);
 /**
  * SQLite column definition
  *
@@ -95,7 +113,7 @@ class DboSqlite extends DboSource {
 		$this->connected = is_resource($this->connection);
 
 		if ($this->connected) {
-			$this->_execute('PRAGMA count_changes = 1');
+			$this->_execute('PRAGMA count_changes = 1;');
 		}
 		return $this->connected;
 	}
@@ -116,7 +134,24 @@ class DboSqlite extends DboSource {
  * @return resource Result resource identifier
  */
 	function _execute($sql) {
-		return sqlite_query($this->connection, $sql);
+		$result = sqlite_query($this->connection, $sql);
+
+		if (preg_match('/^(INSERT|UPDATE|DELETE)/', $sql)) {
+			$this->resultSet($result);
+			list($this->_queryStats) = $this->fetchResult();
+		}
+		return $result;
+	}
+/**
+ * Overrides DboSource::execute() to correctly handle query statistics
+ *
+ * @param string $sql
+ * @return unknown
+ */
+	function execute($sql) {
+		$result = parent::execute($sql);
+		$this->_queryStats = array();
+		return $result;
 	}
 /**
  * Returns an array of tables in the database. If there are no tables, an error is raised and the application exits.
@@ -161,7 +196,7 @@ class DboSqlite extends DboSource {
 			return $cache;
 		}
 		$fields = array();
-		$result = $this->fetchAll('PRAGMA table_info(' . $model->tablePrefix . $model->table . ')');
+		$result = $this->fetchAll('PRAGMA table_info(' . $this->fullTableName($model) . ')');
 
 		foreach ($result as $column) {
 			$fields[$column[0]['name']] = array(
@@ -232,52 +267,8 @@ class DboSqlite extends DboSource {
 				}
 			}
 		}
-		return parent::update($model, $fields, $values, $conditions);
+		$result = parent::update($model, $fields, $values, $conditions);
-	}
+		return $result;
-/**
- * Begin a transaction
- *
- * @param unknown_type $model
- * @return boolean True on success, false on fail
- * (i.e. if the database/model does not support transactions).
- */
-	function begin(&$model) {
-		if (parent::begin($model)) {
-			if ($this->execute('BEGIN')) {
-				$this->_transactionStarted = true;
-				return true;
-			}
-		}
-		return false;
-	}
-/**
- * Commit a transaction
- *
- * @param unknown_type $model
- * @return boolean True on success, false on fail
- * (i.e. if the database/model does not support transactions,
- * or a transaction has not started).
- */
-	function commit(&$model) {
-		if (parent::commit($model)) {
-			$this->_transactionStarted = false;
-			return $this->execute('COMMIT');
-		}
-		return false;
-	}
-/**
- * Rollback a transaction
- *
- * @param unknown_type $model
- * @return boolean True on success, false on fail
- * (i.e. if the database/model does not support transactions,
- * or a transaction has not started).
- */
-	function rollback(&$model) {
-		if (parent::rollback($model)) {
-			return $this->execute('ROLLBACK');
-		}
-		return false;
 	}
 /**
  * Deletes all the records in a table and resets the count of the auto-incrementing
@@ -308,8 +299,12 @@ class DboSqlite extends DboSource {
  * @return integer Number of affected rows
  */
 	function lastAffected() {
-		if ($this->_result) {
+		if (!empty($this->_queryStats)) {
-			return sqlite_changes($this->connection);
+			foreach (array('rows inserted', 'rows updated', 'rows deleted') as $key) {
+				if (array_key_exists($key, $this->_queryStats)) {
+					return $this->_queryStats[$key];
+				}
+			}
 		}
 		return false;
 	}
@@ -374,11 +369,10 @@ class DboSqlite extends DboSource {
 	function resultSet(&$results) {
 		$this->results =& $results;
 		$this->map = array();
-		$num_fields = sqlite_num_fields($results);
+		$fieldCount = sqlite_num_fields($results);
-		$index = 0;
+		$index = $j = 0;
-		$j = 0;
 
-		while ($j < $num_fields) {
+		while ($j < $fieldCount) {
 			$columnName = str_replace('"', '', sqlite_field_name($results, $j));
 
 			if (strpos($columnName, '.')) {

cake/libs/model/datasources/dbo_source.php

@@ -85,6 +85,17 @@ class DboSource extends DataSource {
  * @var array
  */
 	var $__sqlOps = array('like', 'ilike', 'or', 'not', 'in', 'between', 'regexp', 'similar to');
+/**
+ * Index of basic SQL commands
+ *
+ * @var array
+ * @access protected
+ */
+	var $_commands = array(
+		'begin'    => 'BEGIN',
+		'commit'   => 'COMMIT',
+		'rollback' => 'ROLLBACK'
+	);
 /**
  * Constructor
  */
@@ -156,10 +167,10 @@ class DboSource extends DataSource {
 	function execute($sql) {
 		$t = getMicrotime();
 		$this->_result = $this->_execute($sql);
-		$this->affected = $this->lastAffected();
 		$this->took = round((getMicrotime() - $t) * 1000, 0);
+		$this->affected = $this->lastAffected();
 		$this->error = $this->lastError();
-		$this->numRows = $this->lastNumRows($this->_result);
+		$this->numRows = $this->lastNumRows();
 
 		if (Configure::read() > 1) {
 			$this->logQuery($sql);
@@ -276,7 +287,7 @@ class DboSource extends DataSource {
 		}
 	}
 /**
- * Returns a row from current resultset as an array .
+ * Returns a row from current resultset as an array
  *
  * @return array The fetched row as an array
  */
@@ -470,7 +481,7 @@ class DboSource extends DataSource {
 			$sql = substr($sql, 0, 200) . '[...]';
 		}
 
-		if (($error) && Configure::read() > 1) {
+		if (($error) || Configure::read() > 1) {
 			e("<p style = \"text-align:left\"><b>Query:</b> {$sql} ");
 			if ($error) {
 				trigger_error("<span style = \"color:Red;text-align:left\"><b>SQL Error:</b> {$this->error}</span>", E_USER_WARNING);
@@ -1450,6 +1461,21 @@ class DboSource extends DataSource {
 	function truncate($table) {
 		return $this->execute('TRUNCATE TABLE ' . $this->fullTableName($table));
 	}
+/**
+ * Begin a transaction
+ *
+ * @param model $model
+ * @return boolean True on success, false on fail
+ * (i.e. if the database/model does not support transactions,
+ * or a transaction has not started).
+ */
+	function begin(&$model) {
+		if (parent::begin($model) && $this->execute($this->_commands['begin'])) {
+			$this->_transactionStarted = true;
+			return true;
+		}
+		return false;
+	}
 /**
  * Commit a transaction
  *
@@ -1459,7 +1485,7 @@ class DboSource extends DataSource {
  * or a transaction has not started).
  */
 	function commit(&$model) {
-		if (parent::commit($model) && $this->execute('COMMIT')) {
+		if (parent::commit($model) && $this->execute($this->_commands['commit'])) {
 			$this->_transactionStarted = false;
 			return true;
 		}
@@ -1474,7 +1500,7 @@ class DboSource extends DataSource {
  * or a transaction has not started).
  */
 	function rollback(&$model) {
-		if (parent::rollback($model) && $this->execute('ROLLBACK')) {
+		if (parent::rollback($model) && $this->execute($this->_commands['rollback'])) {
 			$this->_transactionStarted = false;
 			return true;
 		}

cake/tests/cases/libs/model/model.test.php

@@ -936,6 +936,8 @@ class ModelTest extends CakeTestCase {
 	}
 
 	function testUpdateWithCalculation() {
+		Configure::write('foo', true);
+
 		$this->loadFixtures('DataTest');
 		$model =& new DataTest();
 		$result = $model->saveAll(array(
@@ -956,6 +958,7 @@ class ModelTest extends CakeTestCase {
 		$this->assertTrue($model->updateAll(array('DataTest.count' => 'DataTest.count - 1')));
 		$result = Set::extract('/DataTest/count', $model->find('all', array('fields' => 'count')));
 		$this->assertEqual($result, array(6, 4, 5, 2));
+		Configure::write('foo', false);
 	}
 
 	function testBindUnbind() {
@@ -3582,6 +3585,10 @@ class ModelTest extends CakeTestCase {
 	}
 
 	function testZeroDefaultFieldValue() {
+		$this->skipIf(
+			$this->db->config['driver'] == 'sqlite',
+			'SQLite uses loose typing, this operation is unsupported'
+		);
 		$this->loadFixtures('DataTest');
 		$TestModel =& new DataTest();
 

cake/tests/fixtures/aco_two_fixture.php

@@ -36,7 +36,7 @@ class AcoTwoFixture extends CakeTestFixture {
 	var $name = 'AcoTwo';
 	var $fields = array(
 		'id'		=> array('type' => 'integer', 'key' => 'primary'),
-		'parent_id'	=> array('type' => 'integer', 'length' => 10, 'null' => true),
+		'parent_id'	=> array('type' => 'integer', 'length' => 10, 'null' => true, 'default' => 0),
 		'model'		=> array('type' => 'string', 'default' => ''),
 		'foreign_key' => array('type' => 'integer', 'length' => 10, 'null' => true),
 		'alias'		=> array('type' => 'string', 'default' => ''),