Cleanups to DatabaseMysqlBase
authorAaron Schulz <aschulz@wikimedia.org>
Fri, 16 Sep 2016 03:33:25 +0000 (20:33 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Sat, 17 Sep 2016 06:01:19 +0000 (06:01 +0000)
* Avoid global methods
* Inject global variables
* Remove $wgAllDBsAreLocalhost hack

Change-Id: I54b23654def1f83518764ad697434aebfc6cef73

autoload.php
includes/DefaultSettings.php
includes/db/Database.php [deleted file]
includes/db/DatabaseMysqlBase.php
includes/db/loadbalancer/LBFactoryMW.php
includes/db/loadbalancer/LBFactorySimple.php
includes/libs/rdbms/database/DatabaseBase.php [new file with mode: 0644]
includes/libs/rdbms/lbfactory/LBFactory.php

index eedf7b4..716e56d 100644 (file)
@@ -317,7 +317,7 @@ $wgAutoloadLocalClasses = [
        'DBUnexpectedError' => __DIR__ . '/includes/libs/rdbms/exception/DBError.php',
        'DataUpdate' => __DIR__ . '/includes/deferred/DataUpdate.php',
        'Database' => __DIR__ . '/includes/libs/rdbms/database/Database.php',
-       'DatabaseBase' => __DIR__ . '/includes/db/Database.php',
+       'DatabaseBase' => __DIR__ . '/includes/libs/rdbms/database/DatabaseBase.php',
        'DatabaseInstaller' => __DIR__ . '/includes/installer/DatabaseInstaller.php',
        'DatabaseLag' => __DIR__ . '/maintenance/lag.php',
        'DatabaseLogEntry' => __DIR__ . '/includes/logging/LogEntry.php',
index 3ab8829..135c3e5 100644 (file)
@@ -1835,13 +1835,6 @@ $wgDBmwschema = null;
  */
 $wgSQLiteDataDir = '';
 
-/**
- * Make all database connections secretly go to localhost. Fool the load balancer
- * thinking there is an arbitrarily large cluster of servers to connect to.
- * Useful for debugging.
- */
-$wgAllDBsAreLocalhost = false;
-
 /**
  * Shared database for multiple wikis. Commonly used for storing a user table
  * for single sign-on. The server for this database must be the same as for the
diff --git a/includes/db/Database.php b/includes/db/Database.php
deleted file mode 100644 (file)
index 4f823b5..0000000
+++ /dev/null
@@ -1,225 +0,0 @@
-<?php
-/**
- * @defgroup Database Database
- *
- * This file deals with database interface functions
- * and query specifics/optimisations.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
- * http://www.gnu.org/copyleft/gpl.html
- *
- * @file
- * @ingroup Database
- */
-
-/**
- * Database abstraction object
- * @ingroup Database
- */
-abstract class DatabaseBase extends Database {
-       /**
-        * Boolean, controls output of large amounts of debug information.
-        * @param bool|null $debug
-        *   - true to enable debugging
-        *   - false to disable debugging
-        *   - omitted or null to do nothing
-        *
-        * @return bool Previous value of the flag
-        * @deprecated since 1.28; use setFlag()
-        */
-       public function debug( $debug = null ) {
-               $res = $this->getFlag( DBO_DEBUG );
-               if ( $debug !== null ) {
-                       $debug ? $this->setFlag( DBO_DEBUG ) : $this->clearFlag( DBO_DEBUG );
-               }
-
-               return $res;
-       }
-
-       /**
-        * @return string Command delimiter used by this database engine
-        */
-       public function getDelimiter() {
-               return $this->delimiter;
-       }
-
-       /**
-        * Returns true if this database supports (and uses) cascading deletes
-        *
-        * @return bool
-        */
-       public function cascadingDeletes() {
-               return false;
-       }
-       /**
-        * Returns true if this database supports (and uses) triggers (e.g. on the page table)
-        *
-        * @return bool
-        */
-       public function cleanupTriggers() {
-               return false;
-       }
-       /**
-        * Returns true if this database is strict about what can be put into an IP field.
-        * Specifically, it uses a NULL value instead of an empty string.
-        *
-        * @return bool
-        */
-       public function strictIPs() {
-               return false;
-       }
-
-       /**
-        * Returns true if this database can do a native search on IP columns
-        * e.g. this works as expected: .. WHERE rc_ip = '127.42.12.102/32';
-        *
-        * @return bool
-        */
-       public function searchableIPs() {
-               return false;
-       }
-
-       /**
-        * Returns true if this database uses timestamps rather than integers
-        *
-        * @return bool
-        */
-       public function realTimestamps() {
-               return false;
-       }
-
-       /**
-        * Returns true if this database can use functional indexes
-        *
-        * @return bool
-        */
-       public function functionalIndexes() {
-               return false;
-       }
-
-       /**
-        * Intended to be compatible with the PEAR::DB wrapper functions.
-        * http://pear.php.net/manual/en/package.database.db.intro-execute.php
-        *
-        * ? = scalar value, quoted as necessary
-        * ! = raw SQL bit (a function for instance)
-        * & = filename; reads the file and inserts as a blob
-        *     (we don't use this though...)
-        *
-        * @param string $sql
-        * @param string $func
-        *
-        * @return array
-        */
-       protected function prepare( $sql, $func = __METHOD__ ) {
-               /* MySQL doesn't support prepared statements (yet), so just
-                * pack up the query for reference. We'll manually replace
-                * the bits later.
-                */
-               return [ 'query' => $sql, 'func' => $func ];
-       }
-
-       /**
-        * Free a prepared query, generated by prepare().
-        * @param string $prepared
-        */
-       protected function freePrepared( $prepared ) {
-               /* No-op by default */
-       }
-
-       /**
-        * Execute a prepared query with the various arguments
-        * @param string $prepared The prepared sql
-        * @param mixed $args Either an array here, or put scalars as varargs
-        *
-        * @return ResultWrapper
-        */
-       public function execute( $prepared, $args = null ) {
-               if ( !is_array( $args ) ) {
-                       # Pull the var args
-                       $args = func_get_args();
-                       array_shift( $args );
-               }
-
-               $sql = $this->fillPrepared( $prepared['query'], $args );
-
-               return $this->query( $sql, $prepared['func'] );
-       }
-
-       /**
-        * For faking prepared SQL statements on DBs that don't support it directly.
-        *
-        * @param string $preparedQuery A 'preparable' SQL statement
-        * @param array $args Array of Arguments to fill it with
-        * @return string Executable SQL
-        */
-       public function fillPrepared( $preparedQuery, $args ) {
-               reset( $args );
-               $this->preparedArgs =& $args;
-
-               return preg_replace_callback( '/(\\\\[?!&]|[?!&])/',
-                       [ &$this, 'fillPreparedArg' ], $preparedQuery );
-       }
-
-       /**
-        * preg_callback func for fillPrepared()
-        * The arguments should be in $this->preparedArgs and must not be touched
-        * while we're doing this.
-        *
-        * @param array $matches
-        * @throws DBUnexpectedError
-        * @return string
-        */
-       protected function fillPreparedArg( $matches ) {
-               switch ( $matches[1] ) {
-                       case '\\?':
-                               return '?';
-                       case '\\!':
-                               return '!';
-                       case '\\&':
-                               return '&';
-               }
-
-               list( /* $n */, $arg ) = each( $this->preparedArgs );
-
-               switch ( $matches[1] ) {
-                       case '?':
-                               return $this->addQuotes( $arg );
-                       case '!':
-                               return $arg;
-                       case '&':
-                               # return $this->addQuotes( file_get_contents( $arg ) );
-                               throw new DBUnexpectedError(
-                                       $this,
-                                       '& mode is not implemented. If it\'s really needed, uncomment the line above.'
-                               );
-                       default:
-                               throw new DBUnexpectedError(
-                                       $this,
-                                       'Received invalid match. This should never happen!'
-                               );
-               }
-       }
-
-       /**
-        * Get search engine class. All subclasses of this need to implement this
-        * if they wish to use searching.
-        *
-        * @return string
-        */
-       public function getSearchEngine() {
-               return 'SearchEngineDummy';
-       }
-}
index 06511e0..46c6678 100644 (file)
@@ -46,6 +46,11 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
        protected $sslCAPath;
        /** @var string[]|null */
        protected $sslCiphers;
+       /** @var string sql_mode value to send on connection */
+       protected $sqlMode;
+       /** @var bool Use experimental UTF-8 transmission encoding */
+       protected $utf8Mode;
+
        /** @var string|null */
        private $serverVersion = null;
 
@@ -82,6 +87,8 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
                                $this->$var = $params[$var];
                        }
                }
+               $this->sqlMode = isset( $params['sqlMode'] ) ? $params['sqlMode'] : '';
+               $this->utf8Mode = !empty( $params['utf8Mode'] );
        }
 
        /**
@@ -100,13 +107,9 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
         * @return bool
         */
        function open( $server, $user, $password, $dbName ) {
-               global $wgAllDBsAreLocalhost, $wgSQLMode;
-
                # Close/unset connection handle
                $this->close();
 
-               # Debugging hack -- fake cluster
-               $realServer = $wgAllDBsAreLocalhost ? 'localhost' : $server;
                $this->mServer = $server;
                $this->mUser = $user;
                $this->mPassword = $password;
@@ -114,7 +117,7 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
 
                $this->installErrorHandler();
                try {
-                       $this->mConn = $this->mysqlConnect( $realServer );
+                       $this->mConn = $this->mysqlConnect( $this->mServer );
                } catch ( Exception $ex ) {
                        $this->restoreErrorHandler();
                        throw $ex;
@@ -126,14 +129,14 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
                        if ( !$error ) {
                                $error = $this->lastError();
                        }
-                       wfLogDBError(
+                       $this->queryLogger->error(
                                "Error connecting to {db_server}: {error}",
                                $this->getLogContext( [
                                        'method' => __METHOD__,
                                        'error' => $error,
                                ] )
                        );
-                       wfDebug( "DB connection error\n" .
+                       $this->queryLogger->debug( "DB connection error\n" .
                                "Server: $server, User: $user, Password: " .
                                substr( $password, 0, 3 ) . "..., error: " . $error . "\n" );
 
@@ -145,14 +148,14 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
                        $success = $this->selectDB( $dbName );
                        MediaWiki\restoreWarnings();
                        if ( !$success ) {
-                               wfLogDBError(
+                               $this->queryLogger->error(
                                        "Error selecting database {db_name} on server {db_server}",
                                        $this->getLogContext( [
                                                'method' => __METHOD__,
                                        ] )
                                );
-                               wfDebug( "Error selecting database $dbName on server {$this->mServer} " .
-                                       "from client host " . wfHostname() . "\n" );
+                               $this->queryLogger->debug(
+                                       "Error selecting database $dbName on server {$this->mServer}" );
 
                                $this->reportConnectionError( "Error selecting database $dbName" );
                        }
@@ -166,8 +169,8 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
                // Abstract over any insane MySQL defaults
                $set = [ 'group_concat_max_len = 262144' ];
                // Set SQL mode, default is turning them all off, can be overridden or skipped with null
-               if ( is_string( $wgSQLMode ) ) {
-                       $set[] = 'sql_mode = ' . $this->addQuotes( $wgSQLMode );
+               if ( is_string( $this->sqlMode ) ) {
+                       $set[] = 'sql_mode = ' . $this->addQuotes( $this->sqlMode );
                }
                // Set any custom settings defined by site config
                // (e.g. https://dev.mysql.com/doc/refman/4.1/en/innodb-parameters.html)
@@ -183,7 +186,7 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
                        // Use doQuery() to avoid opening implicit transactions (DBO_TRX)
                        $success = $this->doQuery( 'SET ' . implode( ', ', $set ) );
                        if ( !$success ) {
-                               wfLogDBError(
+                               $this->queryLogger->error(
                                        'Error setting MySQL variables on server {db_server} (check $wgSQLMode)',
                                        $this->getLogContext( [
                                                'method' => __METHOD__,
@@ -204,9 +207,7 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
         * @return bool
         */
        protected function connectInitCharset() {
-               global $wgDBmysql5;
-
-               if ( $wgDBmysql5 ) {
+               if ( $this->utf8Mode ) {
                        // Tell the server we're communicating with it in UTF-8.
                        // This may engage various charset conversions.
                        return $this->mysqlSetCharset( 'utf8' );
@@ -657,7 +658,7 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
                        // Standard method: use master server ID (works with stock pt-heartbeat)
                        $masterInfo = $this->getMasterServerInfo();
                        if ( !$masterInfo ) {
-                               wfLogDBError(
+                               $this->queryLogger->error(
                                        "Unable to query master of {db_server} for server ID",
                                        $this->getLogContext( [
                                                'method' => __METHOD__
@@ -680,7 +681,7 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
                        return max( $nowUnix - $timeUnix, 0.0 );
                }
 
-               wfLogDBError(
+               $this->queryLogger->error(
                        "Unable to find pt-heartbeat row for {db_server}",
                        $this->getLogContext( [
                                'method' => __METHOD__
@@ -984,7 +985,7 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
                        return true;
                }
 
-               wfDebug( __METHOD__ . " failed to acquire lock\n" );
+               $this->queryLogger->debug( __METHOD__ . " failed to acquire lock\n" );
 
                return false;
        }
@@ -1006,7 +1007,7 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
                        return true;
                }
 
-               wfDebug( __METHOD__ . " failed to release lock\n" );
+               $this->queryLogger->debug( __METHOD__ . " failed to release lock\n" );
 
                return false;
        }
index 2fb48c7..49d0624 100644 (file)
@@ -37,7 +37,7 @@ abstract class LBFactoryMW extends LBFactory implements DestructibleService {
         * @TODO: inject objects via dependency framework
         */
        public function __construct( array $conf ) {
-               global $wgCommandLineMode;
+               global $wgCommandLineMode, $wgSQLMode, $wgDBmysql5;
 
                $defaults = [
                        'localDomain' => wfWikiID(),
@@ -66,6 +66,11 @@ abstract class LBFactoryMW extends LBFactory implements DestructibleService {
                $this->agent = isset( $params['agent'] ) ? $params['agent'] : '';
                $this->cliMode = isset( $params['cliMode'] ) ? $params['cliMode'] : $wgCommandLineMode;
 
+               if ( isset( $conf['serverTemplate'] ) ) { // LBFactoryMulti
+                       $conf['serverTemplate']['sqlMode'] = $wgSQLMode;
+                       $conf['serverTemplate']['utf8Mode'] = $wgDBmysql5;
+               }
+
                parent::__construct( $conf + $defaults );
        }
 
index 09533eb..d937ddb 100644 (file)
@@ -46,7 +46,7 @@ class LBFactorySimple extends LBFactoryMW {
         * @return LoadBalancer
         */
        public function newMainLB( $wiki = false ) {
-               global $wgDBservers, $wgDBprefix, $wgDBmwschema;
+               global $wgDBservers, $wgDBprefix, $wgDBmwschema, $wgSQLMode, $wgDBmysql5;
 
                if ( is_array( $wgDBservers ) ) {
                        $servers = $wgDBservers;
@@ -59,7 +59,9 @@ class LBFactorySimple extends LBFactoryMW {
                                $server += [
                                        'schema' => $wgDBmwschema,
                                        'tablePrefix' => $wgDBprefix,
-                                       'flags' => DBO_DEFAULT
+                                       'flags' => DBO_DEFAULT,
+                                       'sqlMode' => $wgSQLMode,
+                                       'utf8Mode' => $wgDBmysql5
                                ];
                        }
                } else {
@@ -87,7 +89,9 @@ class LBFactorySimple extends LBFactoryMW {
                                'type' => $wgDBtype,
                                'load' => 1,
                                'flags' => $flags,
-                               'master' => true
+                               'master' => true,
+                               'sqlMode' => $wgSQLMode,
+                               'utf8Mode' => $wgDBmysql5
                        ] ];
                }
 
diff --git a/includes/libs/rdbms/database/DatabaseBase.php b/includes/libs/rdbms/database/DatabaseBase.php
new file mode 100644 (file)
index 0000000..4f823b5
--- /dev/null
@@ -0,0 +1,225 @@
+<?php
+/**
+ * @defgroup Database Database
+ *
+ * This file deals with database interface functions
+ * and query specifics/optimisations.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ * @ingroup Database
+ */
+
+/**
+ * Database abstraction object
+ * @ingroup Database
+ */
+abstract class DatabaseBase extends Database {
+       /**
+        * Boolean, controls output of large amounts of debug information.
+        * @param bool|null $debug
+        *   - true to enable debugging
+        *   - false to disable debugging
+        *   - omitted or null to do nothing
+        *
+        * @return bool Previous value of the flag
+        * @deprecated since 1.28; use setFlag()
+        */
+       public function debug( $debug = null ) {
+               $res = $this->getFlag( DBO_DEBUG );
+               if ( $debug !== null ) {
+                       $debug ? $this->setFlag( DBO_DEBUG ) : $this->clearFlag( DBO_DEBUG );
+               }
+
+               return $res;
+       }
+
+       /**
+        * @return string Command delimiter used by this database engine
+        */
+       public function getDelimiter() {
+               return $this->delimiter;
+       }
+
+       /**
+        * Returns true if this database supports (and uses) cascading deletes
+        *
+        * @return bool
+        */
+       public function cascadingDeletes() {
+               return false;
+       }
+       /**
+        * Returns true if this database supports (and uses) triggers (e.g. on the page table)
+        *
+        * @return bool
+        */
+       public function cleanupTriggers() {
+               return false;
+       }
+       /**
+        * Returns true if this database is strict about what can be put into an IP field.
+        * Specifically, it uses a NULL value instead of an empty string.
+        *
+        * @return bool
+        */
+       public function strictIPs() {
+               return false;
+       }
+
+       /**
+        * Returns true if this database can do a native search on IP columns
+        * e.g. this works as expected: .. WHERE rc_ip = '127.42.12.102/32';
+        *
+        * @return bool
+        */
+       public function searchableIPs() {
+               return false;
+       }
+
+       /**
+        * Returns true if this database uses timestamps rather than integers
+        *
+        * @return bool
+        */
+       public function realTimestamps() {
+               return false;
+       }
+
+       /**
+        * Returns true if this database can use functional indexes
+        *
+        * @return bool
+        */
+       public function functionalIndexes() {
+               return false;
+       }
+
+       /**
+        * Intended to be compatible with the PEAR::DB wrapper functions.
+        * http://pear.php.net/manual/en/package.database.db.intro-execute.php
+        *
+        * ? = scalar value, quoted as necessary
+        * ! = raw SQL bit (a function for instance)
+        * & = filename; reads the file and inserts as a blob
+        *     (we don't use this though...)
+        *
+        * @param string $sql
+        * @param string $func
+        *
+        * @return array
+        */
+       protected function prepare( $sql, $func = __METHOD__ ) {
+               /* MySQL doesn't support prepared statements (yet), so just
+                * pack up the query for reference. We'll manually replace
+                * the bits later.
+                */
+               return [ 'query' => $sql, 'func' => $func ];
+       }
+
+       /**
+        * Free a prepared query, generated by prepare().
+        * @param string $prepared
+        */
+       protected function freePrepared( $prepared ) {
+               /* No-op by default */
+       }
+
+       /**
+        * Execute a prepared query with the various arguments
+        * @param string $prepared The prepared sql
+        * @param mixed $args Either an array here, or put scalars as varargs
+        *
+        * @return ResultWrapper
+        */
+       public function execute( $prepared, $args = null ) {
+               if ( !is_array( $args ) ) {
+                       # Pull the var args
+                       $args = func_get_args();
+                       array_shift( $args );
+               }
+
+               $sql = $this->fillPrepared( $prepared['query'], $args );
+
+               return $this->query( $sql, $prepared['func'] );
+       }
+
+       /**
+        * For faking prepared SQL statements on DBs that don't support it directly.
+        *
+        * @param string $preparedQuery A 'preparable' SQL statement
+        * @param array $args Array of Arguments to fill it with
+        * @return string Executable SQL
+        */
+       public function fillPrepared( $preparedQuery, $args ) {
+               reset( $args );
+               $this->preparedArgs =& $args;
+
+               return preg_replace_callback( '/(\\\\[?!&]|[?!&])/',
+                       [ &$this, 'fillPreparedArg' ], $preparedQuery );
+       }
+
+       /**
+        * preg_callback func for fillPrepared()
+        * The arguments should be in $this->preparedArgs and must not be touched
+        * while we're doing this.
+        *
+        * @param array $matches
+        * @throws DBUnexpectedError
+        * @return string
+        */
+       protected function fillPreparedArg( $matches ) {
+               switch ( $matches[1] ) {
+                       case '\\?':
+                               return '?';
+                       case '\\!':
+                               return '!';
+                       case '\\&':
+                               return '&';
+               }
+
+               list( /* $n */, $arg ) = each( $this->preparedArgs );
+
+               switch ( $matches[1] ) {
+                       case '?':
+                               return $this->addQuotes( $arg );
+                       case '!':
+                               return $arg;
+                       case '&':
+                               # return $this->addQuotes( file_get_contents( $arg ) );
+                               throw new DBUnexpectedError(
+                                       $this,
+                                       '& mode is not implemented. If it\'s really needed, uncomment the line above.'
+                               );
+                       default:
+                               throw new DBUnexpectedError(
+                                       $this,
+                                       'Received invalid match. This should never happen!'
+                               );
+               }
+       }
+
+       /**
+        * Get search engine class. All subclasses of this need to implement this
+        * if they wish to use searching.
+        *
+        * @return string
+        */
+       public function getSearchEngine() {
+               return 'SearchEngineDummy';
+       }
+}
index e011ff0..49fac6a 100644 (file)
@@ -445,8 +445,7 @@ abstract class LBFactory {
                $failed = [];
                foreach ( $lbs as $i => $lb ) {
                        if ( $masterPositions[$i] ) {
-                               // The DBMS may not support getMasterPos() or the whole
-                               // load balancer might be fake (e.g. $wgAllDBsAreLocalhost).
+                               // The DBMS may not support getMasterPos()
                                if ( !$lb->waitForAll( $masterPositions[$i], $opts['timeout'] ) ) {
                                        $failed[] = $lb->getServerName( $lb->getWriterIndex() );
                                }