From a5053d4c5ad87796864a7030370be19ea44e923c Mon Sep 17 00:00:00 2001 From: Chad Horohoe Date: Mon, 24 Jan 2011 18:36:09 +0000 Subject: [PATCH] * Cleanup massive duplication across Database constructors. Default implementation fairly sane. Now they all share the same if( $server ) logic to allow constructing the class without forcing open a connection (MySQL has done this since at least r15094) * Get rid of intermediate installTables() callback * Actually cache the result of DbInstaller::getConnection() like the documentation says --- includes/db/DatabaseIbm_db2.php | 10 +-------- includes/db/DatabaseMssql.php | 7 ------ includes/db/DatabaseOracle.php | 2 +- includes/db/DatabasePostgres.php | 7 ------ includes/db/DatabaseSqlite.php | 10 ++++----- includes/installer/DatabaseInstaller.php | 6 ++++- includes/installer/Installer.php | 19 +--------------- includes/installer/MysqlInstaller.php | 28 ++++++++++++++---------- includes/installer/PostgresInstaller.php | 21 ++++++++++-------- includes/installer/SqliteInstaller.php | 24 +++++++++++--------- 10 files changed, 55 insertions(+), 79 deletions(-) diff --git a/includes/db/DatabaseIbm_db2.php b/includes/db/DatabaseIbm_db2.php index ad2862eedc..329bc6d79e 100644 --- a/includes/db/DatabaseIbm_db2.php +++ b/includes/db/DatabaseIbm_db2.php @@ -264,14 +264,6 @@ class DatabaseIbm_db2 extends DatabaseBase { $dbName = false, $flags = 0, $schema = self::USE_GLOBAL ) { - - global $wgOut, $wgDBmwschema; - # Can't get a reference if it hasn't been set yet - if ( !isset( $wgOut ) ) { - $wgOut = null; - } - $this->mFlags = DBO_TRX | $flags; - if ( $schema == self::USE_GLOBAL ) { $this->mSchema = $wgDBmwschema; } else { @@ -286,7 +278,7 @@ class DatabaseIbm_db2 extends DatabaseBase { $this->setDB2Option( 'rowcount', 'DB2_ROWCOUNT_PREFETCH_ON', self::STMT_OPTION ); - $this->open( $server, $user, $password, $dbName ); + parent::__construct( $server, $user, $password, $dbName, $flags ); } /** diff --git a/includes/db/DatabaseMssql.php b/includes/db/DatabaseMssql.php index 5b3b335df0..af263c77d6 100644 --- a/includes/db/DatabaseMssql.php +++ b/includes/db/DatabaseMssql.php @@ -17,13 +17,6 @@ class DatabaseMssql extends DatabaseBase { var $mLastResult = NULL; var $mAffectedRows = NULL; - function __construct( $server = false, $user = false, $password = false, $dbName = false, - $flags = 0 ) - { - $this->mFlags = $flags; - $this->open( $server, $user, $password, $dbName ); - } - function cascadingDeletes() { return true; } diff --git a/includes/db/DatabaseOracle.php b/includes/db/DatabaseOracle.php index 43ca21ebf6..10a566bb98 100644 --- a/includes/db/DatabaseOracle.php +++ b/includes/db/DatabaseOracle.php @@ -182,7 +182,7 @@ class DatabaseOracle extends DatabaseBase { { $tablePrefix = $tablePrefix == 'get from global' ? $tablePrefix : strtoupper( $tablePrefix ); parent::__construct( $server, $user, $password, $dbName, $flags, $tablePrefix ); - wfRunHooks( 'DatabaseOraclePostInit', array( &$this ) ); + wfRunHooks( 'DatabaseOraclePostInit', array( $this ) ); } function getType() { diff --git a/includes/db/DatabasePostgres.php b/includes/db/DatabasePostgres.php index b36240fb94..151f632b15 100644 --- a/includes/db/DatabasePostgres.php +++ b/includes/db/DatabasePostgres.php @@ -100,13 +100,6 @@ class DatabasePostgres extends DatabaseBase { var $numeric_version = null; var $mAffectedRows = null; - function __construct( $server = false, $user = false, $password = false, $dbName = false, - $flags = 0 ) - { - $this->mFlags = $flags; - $this->open( $server, $user, $password, $dbName ); - } - function getType() { return 'postgres'; } diff --git a/includes/db/DatabaseSqlite.php b/includes/db/DatabaseSqlite.php index 7afccaf3be..60cede3cc5 100644 --- a/includes/db/DatabaseSqlite.php +++ b/includes/db/DatabaseSqlite.php @@ -24,12 +24,12 @@ class DatabaseSqlite extends DatabaseBase { * Parameters $server, $user and $password are not used. */ function __construct( $server = false, $user = false, $password = false, $dbName = false, $flags = 0 ) { - global $wgSharedDB; - $this->mFlags = $flags; $this->mName = $dbName; - - if( $server ) { - if ( $this->open( $server, $user, $password, $dbName ) && $wgSharedDB ) { + parent::__construct( $server, $user, $password, $dbName, $flags ); + // parent doesn't open when $server is false, but we can work with $dbName + if( !$server && $dbName ) { + global $wgSharedDB; + if( $this->open( $server, $user, $password, $dbName ) && $wgSharedDB ) { $this->attachDatabase( $wgSharedDB ); } } diff --git a/includes/installer/DatabaseInstaller.php b/includes/installer/DatabaseInstaller.php index dbdbccff0e..6c4c5527a5 100644 --- a/includes/installer/DatabaseInstaller.php +++ b/includes/installer/DatabaseInstaller.php @@ -28,7 +28,7 @@ abstract class DatabaseInstaller { * * @var DatabaseBase */ - public $db; + public $db = null; /** * Internal variables for installation. @@ -140,6 +140,10 @@ abstract class DatabaseInstaller { } else { $this->db->commit( __METHOD__ ); } + // Resume normal operations + if( $status->isOk() ) { + LBFactory::enableBackend(); + } return $status; } diff --git a/includes/installer/Installer.php b/includes/installer/Installer.php index 25e9fb8925..e83678044b 100644 --- a/includes/installer/Installer.php +++ b/includes/installer/Installer.php @@ -555,23 +555,6 @@ abstract class Installer { $this->parserOptions->setExternalLinkTarget( $wgExternalLinkTarget ); } - /** - * TODO: document - * - * @param $installer DatabaseInstaller - * - * @return Status - */ - public function installTables( DatabaseInstaller &$installer ) { - $status = $installer->createTables(); - - if( $status->isOK() ) { - LBFactory::enableBackend(); - } - - return $status; - } - /** * Exports all wg* variables stored by the installer into global scope. */ @@ -1222,7 +1205,7 @@ abstract class Installer { protected function getInstallSteps( DatabaseInstaller &$installer ) { $coreInstallSteps = array( array( 'name' => 'database', 'callback' => array( $installer, 'setupDatabase' ) ), - array( 'name' => 'tables', 'callback' => array( $this, 'installTables' ) ), + array( 'name' => 'tables', 'callback' => array( $installer, 'createTables' ) ), array( 'name' => 'interwiki', 'callback' => array( $installer, 'populateInterwikiTable' ) ), array( 'name' => 'secretkey', 'callback' => array( $this, 'generateSecretKey' ) ), array( 'name' => 'upgradekey', 'callback' => array( $this, 'generateUpgradeKey' ) ), diff --git a/includes/installer/MysqlInstaller.php b/includes/installer/MysqlInstaller.php index 57f44c104e..aabaccab14 100644 --- a/includes/installer/MysqlInstaller.php +++ b/includes/installer/MysqlInstaller.php @@ -113,19 +113,23 @@ class MysqlInstaller extends DatabaseInstaller { public function getConnection() { $status = Status::newGood(); - try { - $this->db = new DatabaseMysql( - $this->getVar( 'wgDBserver' ), - $this->getVar( '_InstallUser' ), - $this->getVar( '_InstallPassword' ), - false, - false, - 0, - $this->getVar( 'wgDBprefix' ) - ); + if( is_null( $this->db ) ) { + try { + $this->db = new DatabaseMysql( + $this->getVar( 'wgDBserver' ), + $this->getVar( '_InstallUser' ), + $this->getVar( '_InstallPassword' ), + false, + false, + 0, + $this->getVar( 'wgDBprefix' ) + ); + $status->value = $this->db; + } catch ( DBConnectionError $e ) { + $status->fatal( 'config-connection-error', $e->getMessage() ); + } + } else { $status->value = $this->db; - } catch ( DBConnectionError $e ) { - $status->fatal( 'config-connection-error', $e->getMessage() ); } return $status; } diff --git a/includes/installer/PostgresInstaller.php b/includes/installer/PostgresInstaller.php index e02dc2775b..3f7d5beea8 100644 --- a/includes/installer/PostgresInstaller.php +++ b/includes/installer/PostgresInstaller.php @@ -100,16 +100,19 @@ class PostgresInstaller extends DatabaseInstaller { function getConnection($database = 'template1') { $status = Status::newGood(); - - try { - $this->db = new DatabasePostgres( - $this->getVar( 'wgDBserver' ), - $this->getVar( '_InstallUser' ), - $this->getVar( '_InstallPassword' ), - $database ); + if( is_null( $this->db ) ) { + try { + $this->db = new DatabasePostgres( + $this->getVar( 'wgDBserver' ), + $this->getVar( '_InstallUser' ), + $this->getVar( '_InstallPassword' ), + $database ); + $status->value = $this->db; + } catch ( DBConnectionError $e ) { + $status->fatal( 'config-connection-error', $e->getMessage() ); + } + } else { $status->value = $this->db; - } catch ( DBConnectionError $e ) { - $status->fatal( 'config-connection-error', $e->getMessage() ); } return $status; } diff --git a/includes/installer/SqliteInstaller.php b/includes/installer/SqliteInstaller.php index 9393d83183..32d22e67cf 100644 --- a/includes/installer/SqliteInstaller.php +++ b/includes/installer/SqliteInstaller.php @@ -93,17 +93,21 @@ class SqliteInstaller extends DatabaseInstaller { global $wgSQLiteDataDir; $status = Status::newGood(); - $dir = $this->getVar( 'wgSQLiteDataDir' ); - $dbName = $this->getVar( 'wgDBname' ); - - try { - # FIXME: need more sensible constructor parameters, e.g. single associative array - # Setting globals kind of sucks - $wgSQLiteDataDir = $dir; - $this->db = new DatabaseSqlite( false, false, false, $dbName ); + if( is_null( $this->db ) ) { + $dir = $this->getVar( 'wgSQLiteDataDir' ); + $dbName = $this->getVar( 'wgDBname' ); + + try { + # FIXME: need more sensible constructor parameters, e.g. single associative array + # Setting globals kind of sucks + $wgSQLiteDataDir = $dir; + $this->db = new DatabaseSqlite( false, false, false, $dbName ); + $status->value = $this->db; + } catch ( DBConnectionError $e ) { + $status->fatal( 'config-sqlite-connection-error', $e->getMessage() ); + } + } else { $status->value = $this->db; - } catch ( DBConnectionError $e ) { - $status->fatal( 'config-sqlite-connection-error', $e->getMessage() ); } return $status; } -- 2.20.1