Skip to content

Commit

Permalink
Implement QueryFailed and ParameterCountMismatch exceptions
Browse files Browse the repository at this point in the history
These changes are quite invasive towards the database support in
MantisBT. A number of legacy error reporting and handling functions
specific to database handling have been removed.

Legacy database backends (those not using PDO) can be implemented so
that they throw their own exceptions in the same way PDO does. This
moves responsibility over getLastError() type functions into the legacy
backends where it belongs.
  • Loading branch information
davidhicks committed Feb 25, 2012
1 parent c534116 commit a815cc7
Show file tree
Hide file tree
Showing 9 changed files with 115 additions and 140 deletions.
68 changes: 31 additions & 37 deletions application/MantisBT/Db/DriverAbstract.php
Expand Up @@ -19,7 +19,8 @@
namespace MantisBT\Db;

use MantisBT\Db\PDO\Mysql;
use MantisBT\Exception\Db;
use MantisBT\Exception\Database\ParameterCountMismatch;
use MantisBT\Exception\Database\QueryFailed;
use MantisBT\Exception\UnspecifiedException;

/**
Expand Down Expand Up @@ -147,16 +148,10 @@ protected function queryStart( $sql, array $params=null ) {
$this->queries++;
}

/**
* Called immediately after each db query.
* @param mixed db specific result
* @return void
*/
public function queryEnd( $result ) {
if ( $result !== false ) {
return;
}
}
/**
* Called immediately after each db query.
*/
public function queryEnd() {}

/**
* Reset internal column details cache
Expand Down Expand Up @@ -202,36 +197,35 @@ public function perfGetQueries() {
}

/**
* Verify sql parameters
* @param string $sql query or part of it
* @param array $params query parameters
* @return array (sql, params, type of params)
*/
public function checkSqlParameters($sql, array $params=null) {
$params = (array)$params; // make null array if needed

// cast booleans to 1/0 int
foreach ($params as $key => $value) {
$params[$key] = is_bool($value) ? (int)$value : $value;
}
* Verify parameters to SQL query string
* @param string $sql SQL query string (or a portion thereof)
* @param array $parameters Query parameters
* @return array An array containing: [0] = SQL query string, [1] = array of parameters
*/
public function checkSqlParameters($queryString, array $parameters = null) {
$expectedParameterCount = substr_count($queryString, '?');
$actualParameterCount = 0;
if ($parameters !== null) {
$actualParameterCount = count($parameters);
} else {
$parameters = array();
}

$t_count = substr_count($sql, '?');
if ($expectedParameterCount !== $actualParameterCount) {
throw new ParameterCountMismatch($queryString, $parameters);
}

if (!$t_count) {
return array($sql, array() );
}
if ($actualParameterCount === 0) {
return array($queryString, array());
} else {
// cast booleans to 1/0 int
foreach ($parameters as $parameterName => $parameterValue) {
$parameters[$parameterName] = is_bool($parameterValue) ? (int)$parameterValue : $parameterValue;
}

if ($t_count == count($params)) {
return array($sql, array_values($params));
return array($queryString, array_values($parameters));
}

$a = new stdClass;
$a->expected = $t_count;
$a->actual = count($params);
$a->sql = $sql;
$a->params = $params;
throw new Db( ERROR_DB_QUERY_FAILED, $a );
}
}

public function getTableNamePrefix() {
return $this->tableNamePrefix;
Expand Down
15 changes: 4 additions & 11 deletions application/MantisBT/Db/DriverInterface.php
Expand Up @@ -89,24 +89,17 @@ public function dispose();
*/
public function queryStart( $sql, array $params=null );

/**
* Called immediately after each db query.
* @param mixed db specific result
* @return void
*/
public function queryEnd( $result );
/**
* Called immediately after each db query.
*/
public function queryEnd();

/**
* Returns database server info array
* @return array
*/
public function getServerInfo();

/**
* Returns last error reported by database engine.
* @return string error message
*/
public function getLastError();

/**
* Return tables in database WITHOUT current prefix
Expand Down
13 changes: 7 additions & 6 deletions application/MantisBT/Db/PDO/Mysql.php
Expand Up @@ -15,12 +15,14 @@
#
# You should have received a copy of the GNU General Public License
# along with MantisBT. If not, see <http://www.gnu.org/licenses/>.

namespace MantisBT\Db\PDO;
use MantisBT\Db\DriverInterface;
use MantisBT\Db\PDO\PDOAbstract;
use MantisBT\Exception\Db AS DbException;

use \PDO;
use \PDOException;
use MantisBT\Db\DriverInterface;
use MantisBT\Db\PDO\PDOAbstract;
use MantisBT\Exception\Database\QueryFailed;

/**
* MYSQL PDO driver class.
Expand Down Expand Up @@ -86,9 +88,8 @@ public function databaseExists( $name ) {
$sql = "SELECT SCHEMA_NAME FROM INFORMATION_SCHEMA.SCHEMATA WHERE SCHEMA_NAME = ?";
try {
$result = $this->execute( $sql, array( $name ) );
} catch (PDOException $ex) {
throw new DbException(ERROR_DB_QUERY_FAILED, $ex->getMessage());
return false;
} catch (PDOException $exception) {
throw new QueryFailed($exception->getCode(), $exception->getMessage(), $sql);
}
if ($result) {
$value = $result->fetch();
Expand Down
68 changes: 32 additions & 36 deletions application/MantisBT/Db/PDO/PDOAbstract.php
Expand Up @@ -18,12 +18,12 @@

namespace MantisBT\Db\PDO;

use \PDO;
use \PDOException;
use MantisBT\Db\DriverAbstract;
use MantisBT\Exception\Database\ConnectionFailed;
use MantisBT\Exception\Database\DatabaseTypeNotSupported;
use MantisBT\Exception\Db AS DbException;
use \PDO;
use \PDOException;
use MantisBT\Exception\Database\QueryFailed;

/**
* Abstract PDO database driver class.
Expand All @@ -32,8 +32,15 @@
*/
abstract class PDOAbstract extends DriverAbstract {
protected $pdb;
protected $lastError = null;

/**
* Connect to a database using PDO
* @param string $dsn Database Source Name (DSN) for the connection
* @param string $dbHost Hostname or IP address of the database server
* @param string $dbUser Username
* @param string $dbPass Password
* @param array $dbOptions Optional configuration to pass to PDO
*/
public function connect($dsn, $dbHost, $dbUser, $dbPass, $dbName, array $dbOptions = null) {
$driverStatus = $this->driverInstalled();
if ($driverStatus !== true) {
Expand All @@ -46,14 +53,13 @@ public function connect($dsn, $dbHost, $dbUser, $dbPass, $dbName, array $dbOptio
$this->dbName = $dbName;

try {
$this->pdb = new PDO( $this->getDsn(), $this->dbUser, $this->dbPass, $this->getPdoOptions());
$this->pdb = new PDO($this->getDsn(), $this->dbUser, $this->dbPass, $this->getPdoOptions());
$this->pdb->setAttribute(PDO::ATTR_CASE, PDO::CASE_LOWER);
$this->pdb->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$this->postConnect();
} catch (PDOException $exception) {
throw new ConnectionFailed($exception->getCode(), $exception->getMessage());
}
return true;
}

/**
Expand Down Expand Up @@ -121,43 +127,33 @@ public function getInsertId( $table ) {
}
}

/**
* Returns last error reported by database engine.
* @return string error message
*/
public function getLastError() {
return $this->lastError;
}
/**
* Execute SQL query
* @param string $sql Query string
* @param array $params Array of parameters to insert into query string
* @return PDOStatement Resulting handle for the executed prepared statement
*/
public function execute($queryString, array $parameters = null) {
list( $sql, $params ) = $this->checkSqlParameters($queryString, $parameters);
$queryString = $this->remapTableNames($queryString);

/**
* Execute SQL query.
* @param string $sql query
* @param array $params query parameters
* @return bool success
*/
public function execute( $sql, array $params=null ) {
list( $sql, $params ) = $this->checkSqlParameters($sql, $params);
$sql = $this->remapTableNames($sql);
$this->queryStart($queryString, $parameters);

try {
$sth = $this->pdb->prepare($queryString);
$sth->execute($parameters);
} catch (PDOException $exception) {
throw new QueryFailed($exception->getCode(), $exception->getMessage(), $queryString);
}

$result = true;
$this->queryStart( $sql, $params );
$this->queryEnd();

try {
$sth = $this->pdb->prepare( $sql );
$sth->execute( $params );
} catch ( PDOException $ex ) {
$this->lastError = $ex->getMessage();
$result = false;
}

$this->queryEnd( $result );
return $result == true ? $sth : false;
}
return $sth;
}

/**
*/
public function queryStart( $sql, array $params=null ) {
$this->lastError = null;
parent::queryStart( $sql, $params );
}

Expand Down
7 changes: 3 additions & 4 deletions application/MantisBT/Error.php
Expand Up @@ -82,7 +82,7 @@ public static function error_handler( $type, $error, $file, $line, $context ) {
$errorInfo->file = $file;
$errorInfo->line = $line;
$errorInfo->context = $context;
$errorInfo->trace = debug_backtrace();
$errorInfo->trace = debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT);

self::$allErrors[] = $errorInfo;

Expand Down Expand Up @@ -135,8 +135,8 @@ public static function shutdown_error_handler() {
$errorInfo->message = $error['message'];
$errorInfo->file = $error['file'];
$errorInfo->line = $error['line'];
$errorInfo->trace = debug_backtrace();
$errorInfo->context = null;
$errorInfo->trace = debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT);
$errorInfo->context = null;

self::$allErrors[] = $errorInfo;
}
Expand Down Expand Up @@ -295,7 +295,6 @@ public static function error_print_stack_trace( $stack ) {
# remove the call to the error handler from the stack trace
array_shift( $stack );


foreach( $stack as $frame ) {
echo '<tr ', self::error_alternate_class(), '>';
echo '<td>', ( isset( $frame['file'] ) ? htmlentities( $frame['file'], ENT_COMPAT, 'UTF-8' ) : '-' ), '</td><td>', ( isset( $frame['line'] ) ? $frame['line'] : '-' ), '</td><td>', ( isset( $frame['class'] ) ? $frame['class'] : '-' ), '</td><td>', ( isset( $frame['type'] ) ? $frame['type'] : '-' ), '</td><td>', ( isset( $frame['function'] ) ? $frame['function'] : '-' ), '</td>';
Expand Down
18 changes: 18 additions & 0 deletions application/MantisBT/Exception/Database/ParameterCountMismatch.php
@@ -0,0 +1,18 @@
<?php
namespace MantisBT\Exception\Database;
use MantisBT\Exception\ExceptionAbstract;

require_api('lang_api.php');

class ParameterCountMismatch extends ExceptionAbstract {
public function __construct($queryString, array $parameters) {
$expectedParameterCount = substr_count($queryString, '?');
$actualParameterCount = count($parameters);
/* TODO: add new language string */
$errorMessage = lang_get(ERROR_GENERIC, null, false);
$errorMessage = sprintf($errorMessage, $expectedParameterCount, $actualParameterCount, $queryString);
/* TODO: assign new error code instead of 0 */
parent::__construct(0, $errorMessage, null);
$this->responseCode = 500;
}
}
14 changes: 14 additions & 0 deletions application/MantisBT/Exception/Database/QueryFailed.php
@@ -0,0 +1,14 @@
<?php
namespace MantisBT\Exception\Database;
use MantisBT\Exception\ExceptionAbstract;

require_api('lang_api.php');

class QueryFailed extends ExceptionAbstract {
public function __construct($queryErrorCode, $queryErrorDescription, $queryString) {
$errorMessage = lang_get(ERROR_DB_QUERY_FAILED, null, false);
$errorMessage = sprintf($errorMessage, $queryErrorCode, $queryErrorDescription, $queryString);
parent::__construct(ERROR_DB_QUERY_FAILED, $errorMessage, null);
$this->responseCode = 500;
}
}
17 changes: 0 additions & 17 deletions application/MantisBT/Exception/Db.php

This file was deleted.

0 comments on commit a815cc7

Please sign in to comment.