Browse Source

Removing $error property from DboSource, errors in queries will throw exceptions now

Jose Lorenzo Rodriguez 14 years ago
parent
commit
36470f4a86

+ 2 - 1
lib/Cake/Model/Datasource/Database/Mysql.php

@@ -141,7 +141,8 @@ class Mysql extends DboSource {
 		try {
 			$flags = array(
 				PDO::ATTR_PERSISTENT => $config['persistent'],
-				PDO::MYSQL_ATTR_USE_BUFFERED_QUERY => true
+				PDO::MYSQL_ATTR_USE_BUFFERED_QUERY => true,
+				PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
 			);
 			if (!empty($config['encoding'])) {
 				$flags[PDO::MYSQL_ATTR_INIT_COMMAND] = 'SET NAMES ' . $config['encoding'];

+ 2 - 1
lib/Cake/Model/Datasource/Database/Postgres.php

@@ -116,7 +116,8 @@ class Postgres extends DboSource {
 		$this->connected = false;
 		try {
 			$flags = array(
-				PDO::ATTR_PERSISTENT => $config['persistent']
+				PDO::ATTR_PERSISTENT => $config['persistent'],
+				PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
 			);
 			$this->_connection = new PDO(
 				"pgsql:host={$config['host']};port={$config['port']};dbname={$config['database']}",

+ 4 - 2
lib/Cake/Model/Datasource/Database/Sqlite.php

@@ -104,10 +104,12 @@ class Sqlite extends DboSource {
  */
 	public function connect() {
 		$config = $this->config;
-		$flags = array(PDO::ATTR_PERSISTENT => $config['persistent']);
+		$flags = array(
+			PDO::ATTR_PERSISTENT => $config['persistent'],
+			PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
+		);
 		try {
 			$this->_connection = new PDO('sqlite:' . $config['database'], null, null, $flags);
-			$this->_connection->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
 			$this->connected = true;
 		} catch(PDOException $e) {
 			throw new MissingConnectionException(array('class' => $e->getMessage()));

+ 4 - 1
lib/Cake/Model/Datasource/Database/Sqlserver.php

@@ -132,7 +132,10 @@ class Sqlserver extends DboSource {
 		$config = $this->config;
 		$this->connected = false;
 		try {
-			$flags = array(PDO::ATTR_PERSISTENT => $config['persistent']);
+			$flags = array(
+				PDO::ATTR_PERSISTENT => $config['persistent'],
+				PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION
+			);
 			if (!empty($config['encoding'])) {
 				$flags[PDO::SQLSRV_ATTR_ENCODING] = $config['encoding'];
 			}

+ 14 - 69
lib/Cake/Model/Datasource/DboSource.php

@@ -77,13 +77,6 @@ class DboSource extends DataSource {
 	public $fullDebug = false;
 
 /**
- * Error description of last query
- *
- * @var string
- */
-	public $error = null;
-
-/**
  * String to hold how many rows were affected by the last SQL operation.
  *
  * @var string
@@ -383,7 +376,7 @@ class DboSource extends DataSource {
  * @return boolean
  */
 	public function rawQuery($sql, $params = array()) {
-		$this->took = $this->error = $this->numRows = false;
+		$this->took = $this->numRows = false;
 		return $this->execute($sql, $params);
 	}
 
@@ -403,7 +396,6 @@ class DboSource extends DataSource {
  */
 	public function execute($sql, $options = array(), $params = array()) {
 		$options += array('log' => $this->fullDebug);
-		$this->error = null;
 
 		$t = microtime(true);
 		$this->_result = $this->_execute($sql, $params);
@@ -414,10 +406,6 @@ class DboSource extends DataSource {
 			$this->logQuery($sql);
 		}
 
-		if ($this->error) {
-			$this->showQuery($sql);
-			return false;
-		}
 		return $this->_result;
 	}
 
@@ -440,25 +428,18 @@ class DboSource extends DataSource {
 			}
 		}
 
-		try {
-			$query = $this->_connection->prepare($sql, $prepareOptions);
-			$query->setFetchMode(PDO::FETCH_LAZY);
-			if (!$query->execute($params)) {
-				$this->_results = $query;
-				$this->error = $this->lastError($query);
-				$query->closeCursor();
-				return false;
-			}
-			if (!$query->columnCount()) {
-				$query->closeCursor();
-				return true;
-			}
-			return $query;
-		} catch (PDOException $e) {
-			$this->_results = null;
-			$this->error = $e->getMessage();
+		$query = $this->_connection->prepare($sql, $prepareOptions);
+		$query->setFetchMode(PDO::FETCH_LAZY);
+		if (!$query->execute($params)) {
+			$this->_results = $query;
+			$query->closeCursor();
 			return false;
 		}
+		if (!$query->columnCount()) {
+			$query->closeCursor();
+			return true;
+		}
+		return $query;
 	}
 
 /**
@@ -885,7 +866,7 @@ class DboSource extends DataSource {
 			echo $View->element('sql_dump', array('_forced_from_dbo_' => true));
 		} else {
 			foreach ($log['log'] as $k => $i) {
-				print (($k + 1) . ". {$i['query']} {$i['error']}\n");
+				print (($k + 1) . ". {$i['query']}\n");
 			}
 		}
 	}
@@ -894,15 +875,13 @@ class DboSource extends DataSource {
  * Log given SQL query.
  *
  * @param string $sql SQL statement
- * @return void|boolean
- * @todo: Add hook to log errors instead of returning false
+ * @return void
  */
 	public function logQuery($sql) {
 		$this->_queriesCnt++;
 		$this->_queriesTime += $this->took;
 		$this->_queriesLog[] = array(
 			'query'		=> $sql,
-			'error'		=> $this->error,
 			'affected'	=> $this->affected,
 			'numRows'	=> $this->numRows,
 			'took'		=> $this->took
@@ -910,32 +889,6 @@ class DboSource extends DataSource {
 		if (count($this->_queriesLog) > $this->_queriesLogMax) {
 			array_pop($this->_queriesLog);
 		}
-		if ($this->error) {
-			return false;
-		}
-	}
-
-/**
- * Output information about an SQL query. The SQL statement, number of rows in resultset,
- * and execution time in microseconds. If the query fails, an error is output instead.
- *
- * @param string $sql Query to show information on.
- * @return void
- */
-	public function showQuery($sql) {
-		$error = $this->error;
-		if (strlen($sql) > 200 && !$this->fullDebug && Configure::read('debug') > 1) {
-			$sql = substr($sql, 0, 200) . '[...]';
-		}
-		if (Configure::read('debug') > 0) {
-			$out = null;
-			if ($error) {
-				trigger_error('<span style="color:Red;text-align:left"><b>' . __d('cake_dev', 'SQL Error:') . "</b> {$this->error}</span>", E_USER_WARNING);
-			} else {
-				$out = ('<small>[' . __d('cake_dev', 'Aff:%s Num:%s Took:%sms', $this->affected, $this->numRows, $this->took) . ']</small>');
-			}
-			pr(sprintf('<p style="text-align:left"><b>' . __d('cake_dev', 'Query:') . '</b> %s %s</p>', $sql, $out));
-		}
 	}
 
 /**
@@ -1161,16 +1114,8 @@ class DboSource extends DataSource {
 	public function queryAssociation($model, &$linkModel, $type, $association, $assocData, &$queryData, $external = false, &$resultSet, $recursive, $stack) {
 		if ($query = $this->generateAssociationQuery($model, $linkModel, $type, $association, $assocData, $queryData, $external, $resultSet)) {
 			if (!is_array($resultSet)) {
-				if (Configure::read('debug') > 0) {
-					echo '<div style = "font: Verdana bold 12px; color: #FF0000">' . __d('cake_dev', 'SQL Error in model %s:', $model->alias) . ' ';
-					if (isset($this->error) && $this->error != null) {
-						echo $this->error;
-					}
-					echo '</div>';
-				}
-				return null;
+				throw new CakeException(__d('cake_dev', 'Error in Model %s', get_class($model)));
 			}
-
 			if ($type === 'hasMany' && empty($assocData['limit']) && !empty($assocData['foreignKey'])) {
 				$ins = $fetch = array();
 				foreach ($resultSet as &$result) {

+ 5 - 55
lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php

@@ -625,18 +625,8 @@ class DboSourceTest extends CakeTestCase {
 		$result = Set::extract($log['log'], '/query');
 		$expected = array('Query 1', 'Query 2');
 		$this->assertEqual($expected, $result);
-
-		$oldError = $this->testDb->error;
-		$this->testDb->error = true;
-		$result = $this->testDb->logQuery('Error 1');
-		$this->assertFalse($result);
-		$this->testDb->error = $oldError;
-
-		$log = $this->testDb->getLog(false, false);
-		$result = Set::combine($log['log'], '/query', '/error');
-		$expected = array('Query 1' => false, 'Query 2' => false, 'Error 1' => true);
-		$this->assertEqual($expected, $result);
-
+		
+		$oldDebug = Configure::read('debug');
 		Configure::write('debug', 2);
 		ob_start();
 		$this->testDb->showLog();
@@ -644,7 +634,6 @@ class DboSourceTest extends CakeTestCase {
 
 		$this->assertPattern('/Query 1/s', $contents);
 		$this->assertPattern('/Query 2/s', $contents);
-		$this->assertPattern('/Error 1/s', $contents);
 
 		ob_start();
 		$this->testDb->showLog(true);
@@ -652,27 +641,10 @@ class DboSourceTest extends CakeTestCase {
 
 		$this->assertPattern('/Query 1/s', $contents);
 		$this->assertPattern('/Query 2/s', $contents);
-		$this->assertPattern('/Error 1/s', $contents);
 
-		$oldError = $this->testDb->error;
-		$oldDebug = Configure::read('debug');
-		Configure::write('debug', 2);
-
-		$this->testDb->error = $oldError;
 		Configure::write('debug', $oldDebug);
 	}
 
-	public function testShowQueryError() {
-		$this->testDb->error = true;
-		try {
-			$this->testDb->showQuery('Error 2');
-			$this->fail('No exception');
-		} catch (Exception $e) {
-			$this->assertPattern('/SQL Error/', $e->getMessage());
-			$this->assertTrue(true, 'Exception thrown');
-		}
-	}
-
 /**
  * test getting the query log as an array.
  *
@@ -682,19 +654,12 @@ class DboSourceTest extends CakeTestCase {
 		$this->testDb->logQuery('Query 1');
 		$this->testDb->logQuery('Query 2');
 
-		$oldError = $this->testDb->error;
-		$this->testDb->error = true;
-		$result = $this->testDb->logQuery('Error 1');
-		$this->assertFalse($result);
-		$this->testDb->error = $oldError;
-
 		$log = $this->testDb->getLog();
-		$expected = array('query' => 'Query 1', 'error' => '', 'affected' => '', 'numRows' => '', 'took' => '');
+		$expected = array('query' => 'Query 1', 'affected' => '', 'numRows' => '', 'took' => '');
 		$this->assertEqual($log['log'][0], $expected);
-		$expected = array('query' => 'Query 2', 'error' => '', 'affected' => '', 'numRows' => '', 'took' => '');
+		$expected = array('query' => 'Query 2', 'affected' => '', 'numRows' => '', 'took' => '');
 		$this->assertEqual($log['log'][1], $expected);
-		$expected = array('query' => 'Error 1', 'error' => true, 'affected' => '', 'numRows' => '', 'took' => '');
-		$this->assertEqual($log['log'][2], $expected);
+		$expected = array('query' => 'Error 1', 'affected' => '', 'numRows' => '', 'took' => '');
 	}
 
 /**
@@ -713,21 +678,6 @@ class DboSourceTest extends CakeTestCase {
 		$this->assertTrue($result, 'Query did not return a boolean');
 	}
 
-/**
- * test ShowQuery generation of regular and error messages
- *
- * @return void
- */
-	public function testShowQuery() {
-		$this->testDb->error = false;
-		ob_start();
-		$this->testDb->showQuery('Some Query');
-		$contents = ob_get_clean();
-		$this->assertPattern('/Some Query/s', $contents);
-		$this->assertPattern('/Aff:/s', $contents);
-		$this->assertPattern('/Num:/s', $contents);
-		$this->assertPattern('/Took:/s', $contents);
-	}
 
 /**
  * test order to generate query order clause for virtual fields

+ 1 - 0
lib/Cake/View/Elements/sql_dump.ctp

@@ -48,6 +48,7 @@ if ($noLogs || isset($_forced_from_dbo_)):
 	<tbody>
 	<?php
 		foreach ($logInfo['log'] as $k => $i) :
+			$i += array('error' => '');
 			echo "<tr><td>" . ($k + 1) . "</td><td>" . h($i['query']) . "</td><td>{$i['error']}</td><td style = \"text-align: right\">{$i['affected']}</td><td style = \"text-align: right\">{$i['numRows']}</td><td style = \"text-align: right\">{$i['took']}</td></tr>\n";
 		endforeach;
 	?>