From b0239f43302110f250d6121a518c5119a76d1fca Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Fri, 1 Aug 2014 17:56:21 +1200 Subject: [PATCH] BUG Fix PDOConnector issues Travis support for PDO ATTR_EMULATE_PREPARES = false breaks some test cases Enable username sans password Remove unnecessary semicolons delimiting queries --- .travis.yml | 10 ++--- model/connect/MySQLDatabase.php | 33 +++++++------- model/connect/PDOConnector.php | 80 +++++++++++++++++---------------- model/connect/PDOQuery.php | 4 -- 4 files changed, 62 insertions(+), 65 deletions(-) diff --git a/.travis.yml b/.travis.yml index ee5febf23..97db16c9d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,20 +14,20 @@ env: matrix: include: - - php: 5.3 - env: DB=PGSQL CORE_RELEASE=master - - php: 5.3 + - php: 5.4 env: DB=SQLITE CORE_RELEASE=master + - php: 5.4 + env: DB=PGSQL CORE_RELEASE=master - php: 5.4 env: DB=MYSQL CORE_RELEASE=master + - php: 5.4 + env: DB=MYSQL CORE_RELEASE=master PDO=1 - php: 5.5 env: DB=MYSQL CORE_RELEASE=master - php: 5.6 env: DB=MYSQL CORE_RELEASE=master - php: 5.4 env: DB=MYSQL CORE_RELEASE=master BEHAT_TEST=1 - - php: 5.6 - env: DB=MYSQL CORE_RELEASE=master before_script: - composer self-update diff --git a/model/connect/MySQLDatabase.php b/model/connect/MySQLDatabase.php index 01b34d42f..be57bbadf 100644 --- a/model/connect/MySQLDatabase.php +++ b/model/connect/MySQLDatabase.php @@ -9,10 +9,10 @@ * @subpackage model */ class MySQLDatabase extends SS_Database { - + /** * Default connection charset (may be overridden in $databaseConfig) - * + * * @config * @var String */ @@ -23,9 +23,9 @@ class MySQLDatabase extends SS_Database { if(empty($parameters['driver'])) { $parameters['driver'] = $this->getDatabaseServer(); } - + // Set charset - if( empty($parameters['charset']) + if( empty($parameters['charset']) && ($charset = Config::inst()->get('MySQLDatabase', 'connection_charset')) ) { $parameters['charset'] = $charset; @@ -42,13 +42,13 @@ class MySQLDatabase extends SS_Database { } // SS_Database subclass maintains responsibility for selecting database - // once connected in order to correctly handle schema queries about + // once connected in order to correctly handle schema queries about // existence of database, error handling at the correct level, etc if (!empty($parameters['database'])) { $this->selectDatabase($parameters['database'], false, false); } } - + /** * Sets the character set for the MySQL database connection. * @@ -68,7 +68,7 @@ class MySQLDatabase extends SS_Database { /** * Sets the SQL mode - * + * * @param string $mode Connection mode */ public function setSQLMode($mode) { @@ -78,7 +78,7 @@ class MySQLDatabase extends SS_Database { /** * Sets the system timezone for the database connection - * + * * @param string $timezone */ public function selectTimezone($timezone) { @@ -112,7 +112,6 @@ class MySQLDatabase extends SS_Database { if (!class_exists('File')) throw new Exception('MySQLDatabase->searchEngine() requires "File" class'); - $fileFilter = ''; $keywords = $this->escapeString($keywords); $htmlEntityKeywords = htmlentities($keywords, ENT_NOQUOTES, 'UTF-8'); @@ -131,7 +130,7 @@ class MySQLDatabase extends SS_Database { // Always ensure that only pages with ShowInSearch = 1 can be searched $extraFilters['SiteTree'] .= " AND ShowInSearch <> 0"; - // File.ShowInSearch was added later, keep the database driver backwards compatible + // File.ShowInSearch was added later, keep the database driver backwards compatible // by checking for its existence first $fields = $this->fieldList('File'); if (array_key_exists('ShowInSearch', $fields)) @@ -201,7 +200,7 @@ class MySQLDatabase extends SS_Database { $querySQLs[] = $query->sql($parameters); $queryParameters = array_merge($queryParameters, $parameters); - + $totalCount += $query->unlimitedRowCount(); } $fullQuery = implode(" UNION ", $querySQLs) . " ORDER BY $sortBy LIMIT $limit"; @@ -233,30 +232,30 @@ class MySQLDatabase extends SS_Database { public function transactionStart($transactionMode = false, $sessionCharacteristics = false) { // This sets the isolation level for the NEXT transaction, not the current one. if ($transactionMode) { - $this->query('SET TRANSACTION ' . $transactionMode . ';'); + $this->query('SET TRANSACTION ' . $transactionMode); } - $this->query('START TRANSACTION;'); + $this->query('START TRANSACTION'); if ($sessionCharacteristics) { - $this->query('SET SESSION TRANSACTION ' . $sessionCharacteristics . ';'); + $this->query('SET SESSION TRANSACTION ' . $sessionCharacteristics); } } public function transactionSavepoint($savepoint) { - $this->query("SAVEPOINT $savepoint;"); + $this->query("SAVEPOINT $savepoint"); } public function transactionRollback($savepoint = false) { if ($savepoint) { - $this->query('ROLLBACK TO ' . $savepoint . ';'); + $this->query('ROLLBACK TO ' . $savepoint); } else { $this->query('ROLLBACK'); } } public function transactionEnd($chain = false) { - $this->query('COMMIT AND ' . ($chain ? '' : 'NO ') . 'CHAIN;'); + $this->query('COMMIT AND ' . ($chain ? '' : 'NO ') . 'CHAIN'); } public function comparisonClause($field, $value, $exact = false, $negate = false, $caseSensitive = null, diff --git a/model/connect/PDOConnector.php b/model/connect/PDOConnector.php index 117abe02d..a0a38e41d 100644 --- a/model/connect/PDOConnector.php +++ b/model/connect/PDOConnector.php @@ -6,54 +6,54 @@ * @subpackage model */ class PDOConnector extends DBConnector { - + /** * Should ATTR_EMULATE_PREPARES flag be used to emulate prepared statements? - * + * * @config * @var boolean */ private static $emulate_prepare = false; - + /** * The PDO connection instance - * - * @var PDO + * + * @var PDO */ protected $pdoConnection = null; /** * Name of the currently selected database - * + * * @var string */ protected $databaseName = null; /** * The most recent statement returned from PDODatabase->query - * + * * @var PDOStatement */ protected $lastStatement = null; - + /** * List of prepared statements, cached by SQL string * * @var array */ protected $cachedStatements = array(); - + /** * Flush all prepared statements */ public function flushStatements() { $this->cachedStatements = array(); } - + /** * Retrieve a prepared statement for a given SQL string, or return an already prepared version if * one exists for the given query - * + * * @param string $sql * @return PDOStatement */ @@ -66,10 +66,10 @@ class PDOConnector extends DBConnector { } return $this->cachedStatements[$sql]; } - + /** * Is PDO running in emulated mode - * + * * @return boolean */ public static function is_emulate_prepare() { @@ -84,7 +84,7 @@ class PDOConnector extends DBConnector { // requested via selectDatabase $driver = $parameters['driver'] . ":"; $dsn = array(); - + // Typically this is false, but some drivers will request this if($selectDB) { // Specify complete file path immediately following driver (SQLLite3) @@ -92,14 +92,14 @@ class PDOConnector extends DBConnector { $dsn[] = $parameters['filepath']; } elseif(!empty($parameters['database'])) { // Some databases require a selected database at connection (SQLite3, Azure) - if($parameters['driver'] === 'sqlsrv') { + if($parameters['driver'] === 'sqlsrv') { $dsn[] = "Database={$parameters['database']}"; } else { $dsn[] = "dbname={$parameters['database']}"; } } } - + // Syntax for sql server is slightly different if($parameters['driver'] === 'sqlsrv') { $server = $parameters['server']; @@ -128,18 +128,20 @@ class PDOConnector extends DBConnector { // Connection commands to be run on every re-connection $options = array( - PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8', - PDO::ATTR_EMULATE_PREPARES => self::is_emulate_prepare() + PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8' ); + if(self::is_emulate_prepare()) { + $options[PDO::ATTR_EMULATE_PREPARES] = true; + } // May throw a PDOException if fails - if(empty($parameters['username']) || empty($parameters['password'])) { - $this->pdoConnection = new PDO($driver.implode(';', $dsn)); - } else { - $this->pdoConnection = new PDO($driver.implode(';', $dsn), $parameters['username'], - $parameters['password'], $options); - } - + $this->pdoConnection = new PDO( + $driver.implode(';', $dsn), + empty($parameters['username']) ? '' : $parameters['username'], + empty($parameters['password']) ? '' : $parameters['password'], + $options + ); + // Show selected DB if requested if($this->pdoConnection && $selectDB && !empty($parameters['database'])) { $this->databaseName = $parameters['database']; @@ -164,10 +166,10 @@ class PDOConnector extends DBConnector { public function quoteString($value) { return $this->pdoConnection->quote($value); } - + /** * Executes a query that doesn't return a resultset - * + * * @param string $sql * @param string $sql The SQL query to execute * @param integer $errorLevel For errors to this query, raise PHP errors @@ -176,7 +178,7 @@ class PDOConnector extends DBConnector { public function exec($sql, $errorLevel = E_USER_ERROR) { // Check if we should only preview this query if ($this->previewWrite($sql)) return; - + // Reset last statement to prevent interference in case of error $this->lastStatement = null; @@ -213,7 +215,7 @@ class PDOConnector extends DBConnector { return new PDOQuery($this->lastStatement); } - + /** * Determines the PDO::PARAM_* type for a given PHP type string * @param string $phpType Type of object in PHP @@ -241,10 +243,10 @@ class PDOConnector extends DBConnector { user_error("Cannot bind parameter as it is an unsupported type ($phpType)", E_USER_ERROR); } } - + /** * Bind all parameters to a PDOStatement - * + * * @param PDOStatement $statement * @param array $parameters */ @@ -259,7 +261,7 @@ class PDOConnector extends DBConnector { $phpType = $value['type']; $value = $value['value']; } - + // Check type of parameter $type = $this->getPDOParamType($phpType); if($type === PDO::PARAM_STR) $value = strval($value); @@ -268,7 +270,7 @@ class PDOConnector extends DBConnector { $statement->bindValue($index+1, $value, $type); } } - + public function preparedQuery($sql, $parameters, $errorLevel = E_USER_ERROR) { // Check if we should only preview this query if ($this->previewWrite($sql)) return; @@ -276,14 +278,14 @@ class PDOConnector extends DBConnector { // Benchmark query $self = $this; $this->lastStatement = $this->benchmarkQuery($sql, function($sql) use($parameters, $self) { - + // Prepare statement $statement = $self->getOrPrepareStatement($sql); if(!$statement) return null; - + // Inject parameters $self->bindParameters($statement, $parameters); - + // Safely execute the statement $statement->execute($parameters); return $statement; @@ -298,17 +300,17 @@ class PDOConnector extends DBConnector { return new PDOQuery($this->lastStatement); } - + /** * Determine if a resource has an attached error - * + * * @param PDOStatement|PDO $resource the resource to check * @return boolean Flag indicating true if the resource has an error */ protected function hasError($resource) { // No error if no resource if(empty($resource)) return false; - + // If the error code is empty the statement / connection has not been run yet $code = $resource->errorCode(); if(empty($code)) return false; diff --git a/model/connect/PDOQuery.php b/model/connect/PDOQuery.php index de5f65af1..bd166b8a1 100644 --- a/model/connect/PDOQuery.php +++ b/model/connect/PDOQuery.php @@ -27,10 +27,6 @@ class PDOQuery extends SS_Query { $statement->closeCursor(); } - public function __destruct() { - $this->statement->closeCursor(); - } - public function seek($row) { $this->rowNum = $row - 1; return $this->nextRecord();