From 6c7f49f56199a5e4de195731c2d7a8aacd33dd53 Mon Sep 17 00:00:00 2001 From: Igor Wiedler Date: Mon, 14 Feb 2011 00:00:59 +0100 Subject: [task/refactor-db-testcase] Refactor phpbb_database_test_case Move most of the methods to a separate connection manager class. The test case creates a manager to handle database creation, schema loading and more. Most of the methods could be simplified because they can access shared pdo, config and dbms data. PHPBB3-10043 --- tests/bootstrap.php | 1 + tests/test_framework/phpbb_database_test_case.php | 285 +---------------- .../phpbb_database_test_connection_manager.php | 336 +++++++++++++++++++++ 3 files changed, 349 insertions(+), 273 deletions(-) create mode 100644 tests/test_framework/phpbb_database_test_connection_manager.php diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 1fba323277..c729c6e2d8 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -30,3 +30,4 @@ require_once $phpbb_root_path . 'includes/constants.php'; require_once 'test_framework/phpbb_test_case_helpers.php'; require_once 'test_framework/phpbb_test_case.php'; require_once 'test_framework/phpbb_database_test_case.php'; +require_once 'test_framework/phpbb_database_test_connection_manager.php'; diff --git a/tests/test_framework/phpbb_database_test_case.php b/tests/test_framework/phpbb_database_test_case.php index 32d2696f1c..e1b368dcea 100644 --- a/tests/test_framework/phpbb_database_test_case.php +++ b/tests/test_framework/phpbb_database_test_case.php @@ -9,7 +9,7 @@ abstract class phpbb_database_test_case extends PHPUnit_Extensions_Database_TestCase { - private static $already_connected; + static private $already_connected; protected $test_case_helpers; @@ -38,66 +38,6 @@ abstract class phpbb_database_test_case extends PHPUnit_Extensions_Database_Test return $this->test_case_helpers; } - public function get_dbms_data($dbms) - { - $available_dbms = array( - 'firebird' => array( - 'SCHEMA' => 'firebird', - 'DELIM' => ';;', - 'PDO' => 'firebird', - ), - 'mysqli' => array( - 'SCHEMA' => 'mysql_41', - 'DELIM' => ';', - 'PDO' => 'mysql', - ), - 'mysql' => array( - 'SCHEMA' => 'mysql', - 'DELIM' => ';', - 'PDO' => 'mysql', - ), - 'mssql' => array( - 'SCHEMA' => 'mssql', - 'DELIM' => 'GO', - 'PDO' => 'odbc', - ), - 'mssql_odbc'=> array( - 'SCHEMA' => 'mssql', - 'DELIM' => 'GO', - 'PDO' => 'odbc', - ), - 'mssqlnative' => array( - 'SCHEMA' => 'mssql', - 'DELIM' => 'GO', - 'PDO' => 'sqlsrv', - ), - 'oracle' => array( - 'SCHEMA' => 'oracle', - 'DELIM' => '/', - 'PDO' => 'oci', - ), - 'postgres' => array( - 'SCHEMA' => 'postgres', - 'DELIM' => ';', - 'PDO' => 'pgsql', - ), - 'sqlite' => array( - 'SCHEMA' => 'sqlite', - 'DELIM' => ';', - 'PDO' => 'sqlite2', - ), - ); - - if (isset($available_dbms[$dbms])) - { - return $available_dbms[$dbms]; - } - else - { - trigger_error('Database unsupported', E_USER_ERROR); - } - } - public function get_database_config() { if (isset($_SERVER['PHPBB_TEST_DBMS'])) @@ -142,232 +82,26 @@ abstract class phpbb_database_test_case extends PHPUnit_Extensions_Database_Test } } - // NOTE: This function is not the same as split_sql_file from functions_install - public function split_sql_file($sql, $dbms) - { - $dbms_data = $this->get_dbms_data($dbms); - - $sql = str_replace("\r" , '', $sql); - $data = preg_split('/' . preg_quote($dbms_data['DELIM'], '/') . '$/m', $sql); - - $data = array_map('trim', $data); - - // The empty case - $end_data = end($data); - - if (empty($end_data)) - { - unset($data[key($data)]); - } - - if ($dbms == 'sqlite') - { - // remove comment lines starting with # - they are not proper sqlite - // syntax and break sqlite2 - foreach ($data as $i => $query) - { - $data[$i] = preg_replace('/^#.*$/m', "\n", $query); - } - } - - return $data; - } - - /** - * Retrieves a list of all tables from the database. - * - * @param PDO $pdo - * @param string $dbms - * @return array(string) - */ - function get_tables($pdo, $dbms) - { - switch ($pdo) - { - case 'mysql': - case 'mysql4': - case 'mysqli': - $sql = 'SHOW TABLES'; - break; - - case 'sqlite': - $sql = 'SELECT name - FROM sqlite_master - WHERE type = "table"'; - break; - - case 'mssql': - case 'mssql_odbc': - case 'mssqlnative': - $sql = "SELECT name - FROM sysobjects - WHERE type='U'"; - break; - - case 'postgres': - $sql = 'SELECT relname - FROM pg_stat_user_tables'; - break; - - case 'firebird': - $sql = 'SELECT rdb$relation_name - FROM rdb$relations - WHERE rdb$view_source is null - AND rdb$system_flag = 0'; - break; - - case 'oracle': - $sql = 'SELECT table_name - FROM USER_TABLES'; - break; - } - - $result = $pdo->query($sql); - - $tables = array(); - while ($row = $result->fetch(PDO::FETCH_NUM)) - { - $tables[] = current($row); - } - - return $tables; - } - - /** - * Returns a PDO connection for the configured database. - * - * @param array $config The database configuration - * @param array $dbms Information on the used DBMS. - * @param bool $use_db Whether the DSN should be tied to a - * particular database making it impossible - * to delete that database. - * @return PDO The PDO database connection. - */ - public function new_pdo($config, $dbms, $use_db) - { - $dsn = $dbms['PDO'] . ':'; - - switch ($dbms['PDO']) - { - case 'sqlite2': - $dsn .= $config['dbhost']; - break; - - case 'sqlsrv': - // prefix the hostname (or DSN) with Server= so using just (local)\SQLExpress - // works for example, further parameters can still be appended using ;x=y - $dsn .= 'Server='; - // no break -> rest like ODBC - case 'odbc': - // for ODBC assume dbhost is a suitable DSN - // e.g. Driver={SQL Server Native Client 10.0};Server=(local)\SQLExpress; - $dsn .= $config['dbhost']; - - if ($use_db) - { - $dsn .= ';Database=' . $config['dbname']; - } - break; - - default: - $dsn .= 'host=' . $config['dbhost']; - - if ($use_db) - { - $dsn .= ';dbname=' . $config['dbname']; - } - break; - } - - $pdo = new PDO($dsn, $config['dbuser'], $config['dbpasswd']);; - - // good for debug - // $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); - - return $pdo; - } - - private function recreate_db($config, $dbms) - { - switch ($config['dbms']) - { - case 'sqlite': - if (file_exists($config['dbhost'])) - { - unlink($config['dbhost']); - } - break; - - default: - $pdo = $this->new_pdo($config, $dbms, false); - - try - { - $pdo->exec('DROP DATABASE ' . $config['dbname']); - } - catch (PDOException $e) - { - // try to delete all tables if dropping the database was not possible. - foreach ($this->get_tables() as $table) - { - try - { - $pdo->exec('DROP TABLE ' . $table); - } - catch (PDOException $e){} // ignore non-existent tables - } - } - - $pdo->exec('CREATE DATABASE ' . $config['dbname']); - break; - } - } - - private function load_schema($pdo, $config, $dbms) - { - if ($config['dbms'] == 'mysql') - { - $sth = $pdo->query('SELECT VERSION() AS version'); - $row = $sth->fetch(PDO::FETCH_ASSOC); - - if (version_compare($row['version'], '4.1.3', '>=')) - { - $dbms['SCHEMA'] .= '_41'; - } - else - { - $dbms['SCHEMA'] .= '_40'; - } - } - - $sql = $this->split_sql_file(file_get_contents(dirname(__FILE__) . "/../../phpBB/install/schemas/{$dbms['SCHEMA']}_schema.sql"), $config['dbms']); - - foreach ($sql as $query) - { - $pdo->exec($query); - } - } - public function getConnection() { $config = $this->get_database_config(); - $dbms = $this->get_dbms_data($config['dbms']); + + $manager = $this->create_connection_manager($config); if (!self::$already_connected) { - $this->recreate_db($config, $dbms); + $manager->recreate_db(); } - $pdo = $this->new_pdo($config, $dbms, true); + $manager->connect(); if (!self::$already_connected) { - $this->load_schema($pdo, $config, $dbms); - + $manager->load_schema(); self::$already_connected = true; } - return $this->createDefaultDBConnection($pdo, 'testdb'); + return $this->createDefaultDBConnection($manager->get_pdo(), 'testdb'); } public function new_dbal() @@ -399,4 +133,9 @@ abstract class phpbb_database_test_case extends PHPUnit_Extensions_Database_Test { $this->get_test_case_helpers()->setExpectedTriggerError($errno, $message); } + + protected function create_connection_manager($config) + { + return new phpbb_database_test_connection_manager($config); + } } diff --git a/tests/test_framework/phpbb_database_test_connection_manager.php b/tests/test_framework/phpbb_database_test_connection_manager.php new file mode 100644 index 0000000000..e4b306f13f --- /dev/null +++ b/tests/test_framework/phpbb_database_test_connection_manager.php @@ -0,0 +1,336 @@ +config = $config; + $this->dbms = $this->get_dbms_data($this->config['dbms']); + } + + /** + * Return the current PDO instance + */ + public function get_pdo() + { + return $this->pdo; + } + + /** + * Creates a PDO connection for the configured database. + * + * @param bool $use_db Whether the DSN should be tied to a + * particular database making it impossible + * to delete that database. + */ + public function connect($use_db = true) + { + $dsn = $this->dbms['PDO'] . ':'; + + switch ($this->dbms['PDO']) + { + case 'sqlite2': + $dsn .= $this->config['dbhost']; + break; + + case 'sqlsrv': + // prefix the hostname (or DSN) with Server= so using just (local)\SQLExpress + // works for example, further parameters can still be appended using ;x=y + $dsn .= 'Server='; + // no break -> rest like ODBC + case 'odbc': + // for ODBC assume dbhost is a suitable DSN + // e.g. Driver={SQL Server Native Client 10.0};Server=(local)\SQLExpress; + $dsn .= $this->config['dbhost']; + + if ($use_db) + { + $dsn .= ';Database=' . $this->config['dbname']; + } + break; + + default: + $dsn .= 'host=' . $this->config['dbhost']; + + if ($use_db) + { + $dsn .= ';dbname=' . $this->config['dbname']; + } + break; + } + + $this->pdo = new PDO($dsn, $this->config['dbuser'], $this->config['dbpasswd']);; + + // good for debug + // $this->pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + } + + /** + * Load the phpBB database schema into the database + */ + public function load_schema() + { + $this->ensure_connected(__METHOD__); + + $directory = dirname(__FILE__) . '/../../phpBB/install/schemas/'; + $this->load_schema_from_file($directory); + } + + /** + * Drop the database if it exists and re-create it + * + * Note: This does not load the schema, and it is suggested + * to re-connect after calling to get use_db isolation. + */ + public function recreate_db() + { + switch ($this->config['dbms']) + { + case 'sqlite': + if (file_exists($this->config['dbhost'])) + { + unlink($this->config['dbhost']); + } + break; + + default: + $this->connect(false); + + try + { + $this->pdo->exec('DROP DATABASE ' . $config['dbname']); + } + catch (PDOException $e) + { + // try to delete all tables if dropping the database was not possible. + foreach ($this->get_tables() as $table) + { + $this->pdo->exec('DROP TABLE ' . $table); + } + } + + $this->pdo->exec('CREATE DATABASE ' . $this->config['dbname']); + break; + } + } + + /** + * Retrieves a list of all tables from the database. + * + * @return array(string) + */ + public function get_tables() + { + $this->ensure_connected(__METHOD__); + + switch ($this->config['dbms']) + { + case 'mysql': + case 'mysql4': + case 'mysqli': + $sql = 'SHOW TABLES'; + break; + + case 'sqlite': + $sql = 'SELECT name + FROM sqlite_master + WHERE type = "table"'; + break; + + case 'mssql': + case 'mssql_odbc': + case 'mssqlnative': + $sql = "SELECT name + FROM sysobjects + WHERE type='U'"; + break; + + case 'postgres': + $sql = 'SELECT relname + FROM pg_stat_user_tables'; + break; + + case 'firebird': + $sql = 'SELECT rdb$relation_name + FROM rdb$relations + WHERE rdb$view_source is null + AND rdb$system_flag = 0'; + break; + + case 'oracle': + $sql = 'SELECT table_name + FROM USER_TABLES'; + break; + } + + $result = $this->pdo->query($sql); + + $tables = array(); + while ($row = $result->fetch(PDO::FETCH_NUM)) + { + $tables[] = current($row); + } + + return $tables; + } + + /** + * Throw an exception if not connected + */ + protected function ensure_connected($method_name) + { + if (null === $this->pdo) + { + throw new Exception(sprintf('You must connect before calling %s', $method_name)); + } + } + + /** + * Compile the correct schema filename (as per create_schema_files) and + * load it into the database. + */ + protected function load_schema_from_file($directory) + { + $schema = $this->dbms['SCHEMA']; + + if ($this->config['dbms'] == 'mysql') + { + $sth = $this->pdo->query('SELECT VERSION() AS version'); + $row = $sth->fetch(PDO::FETCH_ASSOC); + + if (version_compare($row['version'], '4.1.3', '>=')) + { + $schema .= '_41'; + } + else + { + $schema .= '_40'; + } + } + + $filename = $directory . $schema . '_schema.sql'; + $sql = $this->split_sql(file_get_contents($filename)); + + foreach ($sql as $query) + { + $this->pdo->exec($query); + } + } + + /** + * Split contents of an SQL file into an array of SQL statements + * + * Note: This method is not the same as split_sql_file from functions_install. + * + * @param string $sql Raw contents of an SQL file + * + * @return Array of runnable SQL statements + */ + protected function split_sql($sql) + { + $sql = str_replace("\r" , '', $sql); + $data = preg_split('/' . preg_quote($this->dbms['DELIM'], '/') . '$/m', $sql); + + $data = array_map('trim', $data); + + // The empty case + $end_data = end($data); + + if (empty($end_data)) + { + unset($data[key($data)]); + } + + if ($this->config['dbms'] == 'sqlite') + { + // remove comment lines starting with # - they are not proper sqlite + // syntax and break sqlite2 + foreach ($data as $i => $query) + { + $data[$i] = preg_replace('/^#.*$/m', "\n", $query); + } + } + + return $data; + } + + /** + * Map a phpBB dbms driver name to dbms data array + */ + protected function get_dbms_data($dbms) + { + $available_dbms = array( + 'firebird' => array( + 'SCHEMA' => 'firebird', + 'DELIM' => ';;', + 'PDO' => 'firebird', + ), + 'mysqli' => array( + 'SCHEMA' => 'mysql_41', + 'DELIM' => ';', + 'PDO' => 'mysql', + ), + 'mysql' => array( + 'SCHEMA' => 'mysql', + 'DELIM' => ';', + 'PDO' => 'mysql', + ), + 'mssql' => array( + 'SCHEMA' => 'mssql', + 'DELIM' => 'GO', + 'PDO' => 'odbc', + ), + 'mssql_odbc'=> array( + 'SCHEMA' => 'mssql', + 'DELIM' => 'GO', + 'PDO' => 'odbc', + ), + 'mssqlnative' => array( + 'SCHEMA' => 'mssql', + 'DELIM' => 'GO', + 'PDO' => 'sqlsrv', + ), + 'oracle' => array( + 'SCHEMA' => 'oracle', + 'DELIM' => '/', + 'PDO' => 'oci', + ), + 'postgres' => array( + 'SCHEMA' => 'postgres', + 'DELIM' => ';', + 'PDO' => 'pgsql', + ), + 'sqlite' => array( + 'SCHEMA' => 'sqlite', + 'DELIM' => ';', + 'PDO' => 'sqlite2', + ), + ); + + if (isset($available_dbms[$dbms])) + { + return $available_dbms[$dbms]; + } + else + { + trigger_error('Database unsupported', E_USER_ERROR); + } + } +} -- cgit v1.2.1 From 17b17bc9399c0e72bebb74979f42aae5a683ae8b Mon Sep 17 00:00:00 2001 From: Igor Wiedler Date: Mon, 14 Feb 2011 09:15:51 +0100 Subject: [task/refactor-db-testcase] Improve error message of db tests If database tests cannot be run the error message is ambigous. This commit makes it clearer: - whether the supplied dbms is supported by us - which dbms are supported by us - whether the required PDO extension is loaded PHPBB3-10043 --- .../phpbb_database_test_connection_manager.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/test_framework/phpbb_database_test_connection_manager.php b/tests/test_framework/phpbb_database_test_connection_manager.php index e4b306f13f..3389081c4e 100644 --- a/tests/test_framework/phpbb_database_test_connection_manager.php +++ b/tests/test_framework/phpbb_database_test_connection_manager.php @@ -76,7 +76,14 @@ class phpbb_database_test_connection_manager break; } - $this->pdo = new PDO($dsn, $this->config['dbuser'], $this->config['dbpasswd']);; + try + { + $this->pdo = new PDO($dsn, $this->config['dbuser'], $this->config['dbpasswd']); + } + catch (PDOException $e) + { + throw new Exception("Unable do connect to $dsn with error: {$e->getMessage()}"); + } // good for debug // $this->pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); @@ -330,7 +337,9 @@ class phpbb_database_test_connection_manager } else { - trigger_error('Database unsupported', E_USER_ERROR); + $message = 'Supplied dbms is unsupported, must be one of: '; + $message .= implode(', ', array_keys($available_dbms)); + throw new Exception($message); } } } -- cgit v1.2.1 From d3718bf5d43282cb2136d25b8fd1ba5c8ccf0b17 Mon Sep 17 00:00:00 2001 From: Igor Wiedler Date: Mon, 14 Feb 2011 15:25:05 +0100 Subject: [task/refactor-db-testcase] Do not show db password on connect error PHPBB3-10043 --- tests/test_framework/phpbb_database_test_connection_manager.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_framework/phpbb_database_test_connection_manager.php b/tests/test_framework/phpbb_database_test_connection_manager.php index 3389081c4e..707ce70fd0 100644 --- a/tests/test_framework/phpbb_database_test_connection_manager.php +++ b/tests/test_framework/phpbb_database_test_connection_manager.php @@ -82,7 +82,8 @@ class phpbb_database_test_connection_manager } catch (PDOException $e) { - throw new Exception("Unable do connect to $dsn with error: {$e->getMessage()}"); + $cleaned_dsn = str_replace($this->config['dbpasswd'], '*password*', $dsn); + throw new Exception("Unable do connect to $cleaned_dsn with error: {$e->getMessage()}"); } // good for debug -- cgit v1.2.1 From bd8e5ff79e68e5a270762917af47917f504acbde Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Tue, 15 Feb 2011 21:08:33 -0500 Subject: [task/refactor-db-testcase] Further improve error messages. PHPBB3-10043 --- tests/test_framework/phpbb_database_test_connection_manager.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_framework/phpbb_database_test_connection_manager.php b/tests/test_framework/phpbb_database_test_connection_manager.php index 707ce70fd0..6c06857fbc 100644 --- a/tests/test_framework/phpbb_database_test_connection_manager.php +++ b/tests/test_framework/phpbb_database_test_connection_manager.php @@ -83,7 +83,7 @@ class phpbb_database_test_connection_manager catch (PDOException $e) { $cleaned_dsn = str_replace($this->config['dbpasswd'], '*password*', $dsn); - throw new Exception("Unable do connect to $cleaned_dsn with error: {$e->getMessage()}"); + throw new Exception("Unable do connect to $cleaned_dsn using PDO with error: {$e->getMessage()}"); } // good for debug @@ -338,7 +338,7 @@ class phpbb_database_test_connection_manager } else { - $message = 'Supplied dbms is unsupported, must be one of: '; + $message = "Supplied dbms \"$dbms\" is not a valid phpBB dbms, must be one of: "; $message .= implode(', ', array_keys($available_dbms)); throw new Exception($message); } -- cgit v1.2.1