DatabaseBase constructor cleanups

* The array based format is now required
* Pass any additional parameters through factory() to __construct() even
  if the base clase does not recognize them. This makes DB specific
  options simpler.
* Also added missing bits to Sqlite constructors

Bug: T90288
Change-Id: I29b79a3c1bb2b5e51c1c8f5e04cd08c71e0662a3

diff --git a/RELEASE-NOTES-1.25 b/RELEASE-NOTES-1.25
index cdef220..3dc1933 100644 (file)
--- a/RELEASE-NOTES-1.25
+++ b/RELEASE-NOTES-1.25
@@ -363,6 +363,8 @@ changes to languages because of Bugzilla reports.
 * (T87504) Avoid serving SVG background-images in CSS for Opera 12, which
   renders them incorrectly when combined with border-radius or background-size.
 * Removed maintenance script dumpSisterSites.php.
+* DatabaseBase class constructors must be called using the array argument style.
+  Ideally, DatabaseBase:factory() should be used instead in most cases.
 
 == Compatibility ==
 

diff --git a/includes/db/Database.php b/includes/db/Database.php
index 8ce7f3f..0175e8d 100644 (file)
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -753,32 +753,19 @@ abstract class DatabaseBase implements IDatabase {
         *
         * @param array $params Parameters passed from DatabaseBase::factory()
         */
-       function __construct( $params = null ) {
+       function __construct( array $params ) {
                global $wgDBprefix, $wgDBmwschema, $wgCommandLineMode, $wgDebugDBTransactions;
 
                $this->mTrxAtomicLevels = new SplStack;
 
-               if ( is_array( $params ) ) { // MW 1.22
-                       $server = $params['host'];
-                       $user = $params['user'];
-                       $password = $params['password'];
-                       $dbName = $params['dbname'];
-                       $flags = $params['flags'];
-                       $tablePrefix = $params['tablePrefix'];
-                       $schema = $params['schema'];
-                       $foreign = $params['foreign'];
-               } else { // legacy calling pattern
-                       wfDeprecated( __METHOD__ . " method called without parameter array.", "1.23" );
-                       $args = func_get_args();
-                       $server = isset( $args[0] ) ? $args[0] : false;
-                       $user = isset( $args[1] ) ? $args[1] : false;
-                       $password = isset( $args[2] ) ? $args[2] : false;
-                       $dbName = isset( $args[3] ) ? $args[3] : false;
-                       $flags = isset( $args[4] ) ? $args[4] : 0;
-                       $tablePrefix = isset( $args[5] ) ? $args[5] : 'get from global';
-                       $schema = 'get from global';
-                       $foreign = isset( $args[6] ) ? $args[6] : false;
-               }
+               $server = $params['host'];
+               $user = $params['user'];
+               $password = $params['password'];
+               $dbName = $params['dbname'];
+               $flags = $params['flags'];
+               $tablePrefix = $params['tablePrefix'];
+               $schema = $params['schema'];
+               $foreign = $params['foreign'];
 
                $this->mFlags = $flags;
                if ( $this->mFlags & DBO_DEFAULT ) {
@@ -902,18 +889,17 @@ abstract class DatabaseBase implements IDatabase {
 
                $class = 'Database' . ucfirst( $driver );
                if ( class_exists( $class ) && is_subclass_of( $class, 'DatabaseBase' ) ) {
-                       $params = array(
-                               'host' => isset( $p['host'] ) ? $p['host'] : false,
-                               'user' => isset( $p['user'] ) ? $p['user'] : false,
-                               'password' => isset( $p['password'] ) ? $p['password'] : false,
-                               'dbname' => isset( $p['dbname'] ) ? $p['dbname'] : false,
-                               'flags' => isset( $p['flags'] ) ? $p['flags'] : 0,
-                               'tablePrefix' => isset( $p['tablePrefix'] ) ? $p['tablePrefix'] : 'get from global',
-                               'schema' => isset( $p['schema'] ) ? $p['schema'] : $defaultSchemas[$dbType],
-                               'foreign' => isset( $p['foreign'] ) ? $p['foreign'] : false
-                       );
-
-                       return new $class( $params );
+                       // Resolve some defaults for b/c
+                       $p['host'] = isset( $p['host'] ) ? $p['host'] : false;
+                       $p['user'] = isset( $p['user'] ) ? $p['user'] : false;
+                       $p['password'] = isset( $p['password'] ) ? $p['password'] : false;
+                       $p['dbname'] = isset( $p['dbname'] ) ? $p['dbname'] : false;
+                       $p['flags'] = isset( $p['flags'] ) ? $p['flags'] : 0;
+                       $p['tablePrefix'] = isset( $p['tablePrefix'] ) ? $p['tablePrefix'] : 'get from global';
+                       $p['schema'] = isset( $p['schema'] ) ? $p['schema'] : $defaultSchemas[$dbType];
+                       $p['foreign'] = isset( $p['foreign'] ) ? $p['foreign'] : false;
+
+                       return new $class( $p );
                } else {
                        return null;
                }

diff --git a/includes/db/DatabaseOracle.php b/includes/db/DatabaseOracle.php
index e150206..9b00fbd 100644 (file)
--- a/includes/db/DatabaseOracle.php
+++ b/includes/db/DatabaseOracle.php
@@ -207,23 +207,9 @@ class DatabaseOracle extends DatabaseBase {
        /** @var array */
        private $mFieldInfoCache = array();
 
-       function __construct( $p = null ) {
+       function __construct( array $p ) {
                global $wgDBprefix;
 
-               if ( !is_array( $p ) ) { // legacy calling pattern
-                       wfDeprecated( __METHOD__ . " method called without parameter array.", "1.22" );
-                       $args = func_get_args();
-                       $p = array(
-                               'host' => isset( $args[0] ) ? $args[0] : false,
-                               'user' => isset( $args[1] ) ? $args[1] : false,
-                               'password' => isset( $args[2] ) ? $args[2] : false,
-                               'dbname' => isset( $args[3] ) ? $args[3] : false,
-                               'flags' => isset( $args[4] ) ? $args[4] : 0,
-                               'tablePrefix' => isset( $args[5] ) ? $args[5] : 'get from global',
-                               'schema' => 'get from global',
-                               'foreign' => isset( $args[6] ) ? $args[6] : false
-                       );
-               }
                if ( $p['tablePrefix'] == 'get from global' ) {
                        $p['tablePrefix'] = $wgDBprefix;
                }

diff --git a/includes/db/DatabaseSqlite.php b/includes/db/DatabaseSqlite.php
index 95c44df..7b04716 100644 (file)
--- a/includes/db/DatabaseSqlite.php
+++ b/includes/db/DatabaseSqlite.php
@@ -44,23 +44,9 @@ class DatabaseSqlite extends DatabaseBase {
        /** @var FSLockManager (hopefully on the same server as the DB) */
        protected $lockMgr;
 
-       function __construct( $p = null ) {
+       function __construct( array $p ) {
                global $wgSharedDB, $wgSQLiteDataDir;
 
-               if ( !is_array( $p ) ) { // legacy calling pattern
-                       wfDeprecated( __METHOD__ . " method called without parameter array.", "1.22" );
-                       $args = func_get_args();
-                       $p = array(
-                               'host' => isset( $args[0] ) ? $args[0] : false,
-                               'user' => isset( $args[1] ) ? $args[1] : false,
-                               'password' => isset( $args[2] ) ? $args[2] : false,
-                               'dbname' => isset( $args[3] ) ? $args[3] : false,
-                               'flags' => isset( $args[4] ) ? $args[4] : 0,
-                               'tablePrefix' => isset( $args[5] ) ? $args[5] : 'get from global',
-                               'schema' => 'get from global',
-                               'foreign' => isset( $args[6] ) ? $args[6] : false
-                       );
-               }
                $this->mDBname = $p['dbname'];
                parent::__construct( $p );
                // parent doesn't open when $user is false, but we can work with $dbName
@@ -958,6 +944,11 @@ class DatabaseSqlite extends DatabaseBase {
  */
 class DatabaseSqliteStandalone extends DatabaseSqlite {
        public function __construct( $fileName, $flags = 0 ) {
+               global $wgSQLiteDataDir;
+
+               $this->mTrxAtomicLevels = new SplStack;
+               $this->lockMgr = new FSLockManager( array( 'lockDirectory' => "$wgSQLiteDataDir/locks" ) );
+
                $this->mFlags = $flags;
                $this->tablePrefix( null );
                $this->openFile( $fileName );

diff --git a/includes/installer/SqliteInstaller.php b/includes/installer/SqliteInstaller.php
index 351b022..7e21b4d 100644 (file)
--- a/includes/installer/SqliteInstaller.php
+++ b/includes/installer/SqliteInstaller.php
@@ -188,7 +188,7 @@ class SqliteInstaller extends DatabaseInstaller {
                        # @todo FIXME: Need more sensible constructor parameters, e.g. single associative array
                        # Setting globals kind of sucks
                        $wgSQLiteDataDir = $dir;
-                       $db = new DatabaseSqlite( false, false, false, $dbName );
+                       $db = DatabaseBase::factory( 'sqlite', array( 'dbname' => $dbName ) );
                        $status->value = $db;
                } catch ( DBConnectionError $e ) {
                        $status->fatal( 'config-sqlite-connection-error', $e->getMessage() );