Skip to content

Commit

Permalink
Add params option to logQuery()
Browse files Browse the repository at this point in the history 
Parameters for prepared statements are now part of the
logged query data.
  • Loading branch information
@ichikaway @markstory
ichikaway authored and markstory committed Feb 9, 2012
1 parent 59ff514 commit e8a9d93
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 32 deletions.
5 changes: 3 additions & 2 deletions lib/Cake/Model/Datasource/DboSource.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ public function execute($sql, $options = array(), $params = array()) {
if ($options['log']) {
$this->took = round((microtime(true) - $t) * 1000, 0);
$this->numRows = $this->affected = $this->lastAffected();
$this->logQuery($sql);
$this->logQuery($sql, $params);
}

return $this->_result;
Expand Down Expand Up @@ -894,11 +894,12 @@ public function showLog($sorted = false) {
* @param string $sql SQL statement
* @return void
*/
public function logQuery($sql) {
public function logQuery($sql, $params = array()) {
$this->_queriesCnt++;
$this->_queriesTime += $this->took;
$this->_queriesLog[] = array(
'query' => $sql,
'params' => $params,
'affected' => $this->affected,
'numRows' => $this->numRows,
'took' => $this->took
Expand Down
76 changes: 47 additions & 29 deletions lib/Cake/Test/Case/Model/Datasource/DboSourceTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -651,14 +651,32 @@ public function testGetLog() {
$this->testDb->logQuery('Query 2');

$log = $this->testDb->getLog();
$expected = array('query' => 'Query 1', 'params' => array(), 'affected' => '', 'numRows' => '', 'took' => '');

$expected = array('query' => 'Query 1', 'affected' => '', 'numRows' => '', 'took' => '');
$this->assertEquals($log['log'][0], $expected);
$expected = array('query' => 'Query 2', 'affected' => '', 'numRows' => '', 'took' => '');
$expected = array('query' => 'Query 2', 'params' => array(), 'affected' => '', 'numRows' => '', 'took' => '');
$this->assertEquals($log['log'][1], $expected);
$expected = array('query' => 'Error 1', 'affected' => '', 'numRows' => '', 'took' => '');
}


/**
* test getting the query log as an array, setting bind params.
*
* @return void
*/
public function testGetLogParams() {
$this->testDb->logQuery('Query 1', array(1,2,'abc'));
$this->testDb->logQuery('Query 2', array('field1' => 1, 'field2' => 'abc'));

$log = $this->testDb->getLog();
$expected = array('query' => 'Query 1', 'params' => array(1,2,'abc'), 'affected' => '', 'numRows' => '', 'took' => '');
$this->assertEquals($log['log'][0], $expected);
$expected = array('query' => 'Query 2', 'params' => array('field1' => 1, 'field2' => 'abc'), 'affected' => '', 'numRows' => '', 'took' => '');
$this->assertEquals($log['log'][1], $expected);
}


/**
* test that query() returns boolean values from operations like CREATE TABLE
*
Expand Down Expand Up @@ -797,32 +815,32 @@ function testLastError() {
* @return void
**/
public function testTransactionLogging() {
$conn = $this->getMock('MockPDO');
$db = new DboTestSource;
$db->setConnection($conn);
$conn->expects($this->exactly(2))->method('beginTransaction')
->will($this->returnValue(true));
$conn->expects($this->once())->method('commit')->will($this->returnValue(true));
$conn->expects($this->once())->method('rollback')->will($this->returnValue(true));

$db->begin();
$log = $db->getLog();
$expected = array('query' => 'BEGIN', 'affected' => '', 'numRows' => '', 'took' => '');
$this->assertEquals($expected, $log['log'][0]);

$db->commit();
$expected = array('query' => 'COMMIT', 'affected' => '', 'numRows' => '', 'took' => '');
$log = $db->getLog();
$this->assertEquals($expected, $log['log'][0]);

$db->begin();
$expected = array('query' => 'BEGIN', 'affected' => '', 'numRows' => '', 'took' => '');
$log = $db->getLog();
$this->assertEquals($expected, $log['log'][0]);

$db->rollback();
$expected = array('query' => 'ROLLBACK', 'affected' => '', 'numRows' => '', 'took' => '');
$log = $db->getLog();
$this->assertEquals($expected, $log['log'][0]);
$conn = $this->getMock('MockPDO');
$db = new DboTestSource;
$db->setConnection($conn);
$conn->expects($this->exactly(2))->method('beginTransaction')
->will($this->returnValue(true));
$conn->expects($this->once())->method('commit')->will($this->returnValue(true));
$conn->expects($this->once())->method('rollback')->will($this->returnValue(true));

$db->begin();
$log = $db->getLog();
$expected = array('query' => 'BEGIN', 'params' => array(), 'affected' => '', 'numRows' => '', 'took' => '');
$this->assertEquals($expected, $log['log'][0]);

$db->commit();
$expected = array('query' => 'COMMIT', 'params' => array(), 'affected' => '', 'numRows' => '', 'took' => '');
$log = $db->getLog();
$this->assertEquals($expected, $log['log'][0]);

$db->begin();
$expected = array('query' => 'BEGIN', 'params' => array(), 'affected' => '', 'numRows' => '', 'took' => '');
$log = $db->getLog();
$this->assertEquals($expected, $log['log'][0]);

$db->rollback();
$expected = array('query' => 'ROLLBACK', 'params' => array(), 'affected' => '', 'numRows' => '', 'took' => '');
$log = $db->getLog();
$this->assertEquals($expected, $log['log'][0]);
}
}
15 changes: 14 additions & 1 deletion lib/Cake/View/Elements/sql_dump.ctp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,20 @@ if ($noLogs || isset($_forced_from_dbo_)):
<?php
foreach ($logInfo['log'] as $k => $i) :
$i += array('error' => '');
if(!empty($i['params']) && is_array($i['params'])) {

This comment has been minimized.

Sign in to view

Copy link
@ADmad

ADmad Feb 9, 2012

Member

@ichikaway Please follow cake's coding standards. A space is needed after function name. ie. After each "if" and "foreach" in this patch.
$bindParam = $bindType = null;
if(preg_match('/.+ :.+/', $i['query'])) {
$bindType = true;
}
foreach($i['params'] as $bindKey => $bindVal) {
if($bindType === true) {
$bindParam .= h($bindKey) ." => " . h($bindVal) . ", ";
} else {
$bindParam .= h($bindVal) . ", ";
}
}
$i['query'] .= " , params[ " . rtrim($bindParam, ', ') . " ]";
}
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 All @@ -58,4 +72,3 @@ if ($noLogs || isset($_forced_from_dbo_)):
else:
echo '<p>Encountered unexpected $logs cannot generate SQL log</p>';
endif;
?>

6 comments on commit e8a9d93

@markstory
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can fix that, I merged it in.

@ADmad
Copy link
Member

@ADmad ADmad commented on e8a9d93 Feb 10, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@markstory Thanks. Though this commit has started causing errors in debugkit. DebugKit's, HtmlToobarHelper::table() passes each record of this log to HtmlHelper::tableCells(). Since the value for new 'params' key is an array it goes into this if condition thereby causing undefined index notices.

@ichikaway
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@markstory Thank you
@ADmad Sorry, I will follow cake's coding standards next time.

@markstory
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stupid debug kit. I'll fix that too :)

@markstory
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed DebugKit in cakephp/debug_kit@0b21dae

@ADmad
Copy link
Member

@ADmad ADmad commented on e8a9d93 Feb 11, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Please sign in to comment.