Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Removing $error property from DboSource, errors in queries will throw…
… exceptions now
  • Loading branch information
lorenzo committed Sep 3, 2011
1 parent 783b5d4 commit 36470f4
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 129 deletions.
3 changes: 2 additions & 1 deletion lib/Cake/Model/Datasource/Database/Mysql.php
Expand Up @@ -141,7 +141,8 @@ public function connect() {
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'];
Expand Down
3 changes: 2 additions & 1 deletion lib/Cake/Model/Datasource/Database/Postgres.php
Expand Up @@ -116,7 +116,8 @@ public function connect() {
$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']}",
Expand Down
6 changes: 4 additions & 2 deletions lib/Cake/Model/Datasource/Database/Sqlite.php
Expand Up @@ -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()));
Expand Down
5 changes: 4 additions & 1 deletion lib/Cake/Model/Datasource/Database/Sqlserver.php
Expand Up @@ -132,7 +132,10 @@ public function connect() {
$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'];
}
Expand Down
83 changes: 14 additions & 69 deletions lib/Cake/Model/Datasource/DboSource.php
Expand Up @@ -76,13 +76,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.
*
Expand Down Expand Up @@ -383,7 +376,7 @@ public function expression($expression) {
* @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);
}

Expand All @@ -403,7 +396,6 @@ public function rawQuery($sql, $params = array()) {
*/
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);
Expand All @@ -414,10 +406,6 @@ public function execute($sql, $options = array(), $params = array()) {
$this->logQuery($sql);
}

if ($this->error) {
$this->showQuery($sql);
return false;
}
return $this->_result;
}

Expand All @@ -440,25 +428,18 @@ protected function _execute($sql, $params = array(), $prepareOptions = array())
}
}

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;
}

/**
Expand Down Expand Up @@ -885,7 +866,7 @@ public function showLog($sorted = false) {
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");
}
}
}
Expand All @@ -894,48 +875,20 @@ public function showLog($sorted = false) {
* 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
);
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));
}
}

/**
Expand Down Expand Up @@ -1161,16 +1114,8 @@ protected function _filterResults(&$results, Model $model, $filtered = array())
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) {
Expand Down
60 changes: 5 additions & 55 deletions lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php
Expand Up @@ -625,54 +625,26 @@ public function testLog() {
$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();
$contents = ob_get_clean();

$this->assertPattern('/Query 1/s', $contents);
$this->assertPattern('/Query 2/s', $contents);
$this->assertPattern('/Error 1/s', $contents);

ob_start();
$this->testDb->showLog(true);
$contents = ob_get_clean();

$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.
*
Expand All @@ -682,19 +654,12 @@ public function testGetLog() {
$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' => '');
}

/**
Expand All @@ -713,21 +678,6 @@ public function testFetchAllBooleanReturns() {
$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
Expand Down
1 change: 1 addition & 0 deletions lib/Cake/View/Elements/sql_dump.ctp
Expand Up @@ -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;
?>
Expand Down

0 comments on commit 36470f4

Please sign in to comment.