Avoid MWException and wf* log methods in DatabaseBase

Change-Id: Idc418ae1088f87d6416e2552976d94f7d1e8f5db

diff --git a/includes/db/Database.php b/includes/db/Database.php
index e68a8f2..0c357cc 100644 (file)
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -69,6 +69,8 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
        protected $connLogger;
        /** @var LoggerInterface */
        protected $queryLogger;
+       /** @var callback Error logging callback */
+       protected $errorLogger;
 
        /** @var resource Database connection */
        protected $mConn = null;
@@ -319,7 +321,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
         * @param array $p An array of options to pass to the constructor.
         *    Valid options are: host, user, password, dbname, flags, tablePrefix, schema, driver
         * @return IDatabase|null If the database driver or extension cannot be found
-        * @throws MWException
+        * @throws InvalidArgumentException If the database driver or extension cannot be found
         */
        final public static function factory( $dbType, $p = [] ) {
                global $wgCommandLineMode;
@@ -355,7 +357,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
                        $driver = $dbType;
                }
                if ( $driver === false ) {
-                       throw new MWException( __METHOD__ .
+                       throw new InvalidArgumentException( __METHOD__ .
                                " no viable database extension found for type '$dbType'" );
                }
 
@@ -390,6 +392,11 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
                        if ( isset( $p['queryLogger'] ) ) {
                                $conn->queryLogger = $p['queryLogger'];
                        }
+                       if ( isset( $p['errorLogger'] ) ) {
+                               $conn->errorLogger = $p['errorLogger'];
+                       } else {
+                               $conn->errorLogger = [ MWExceptionHandler::class, 'logException' ];
+                       }
                } else {
                        $conn = null;
                }
@@ -741,7 +748,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
        protected function installErrorHandler() {
                $this->mPHPError = false;
                $this->htmlErrors = ini_set( 'html_errors', '0' );
-               set_error_handler( [ $this, 'connectionErrorHandler' ] );
+               set_error_handler( [ $this, 'connectionerrorLogger' ] );
        }
 
        /**
@@ -766,12 +773,12 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
         * @param int $errno
         * @param string $errstr
         */
-       public function connectionErrorHandler( $errno, $errstr ) {
+       public function connectionerrorLogger( $errno, $errstr ) {
                $this->mPHPError = $errstr;
        }
 
        /**
-        * Create a log context to pass to wfLogDBError or other logging functions.
+        * Create a log context to pass to PSR logging functions.
         *
         * @param array $extras Additional data to add to context
         * @return array
@@ -790,11 +797,6 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
        public function close() {
                if ( $this->mConn ) {
                        if ( $this->trxLevel() ) {
-                               if ( !$this->mTrxAutomatic ) {
-                                       wfWarn( "Transaction still in progress (from {$this->mTrxFname}), " .
-                                               " performing implicit commit before closing connection!" );
-                               }
-
                                $this->commit( __METHOD__, self::FLUSHING_INTERNAL );
                        }
 
@@ -948,7 +950,8 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
                        if ( $this->reconnect() ) {
                                $msg = __METHOD__ . ": lost connection to {$this->getServer()}; reconnected";
                                $this->connLogger->warning( $msg );
-                               $this->queryLogger->warning( "$msg:\n" . wfBacktrace( true ) );
+                               $this->queryLogger->warning(
+                                       "$msg:\n" . ( new RuntimeException() )->getTraceAsString() );
 
                                if ( !$recoverable ) {
                                        # Callers may catch the exception and continue to use the DB
@@ -1097,7 +1100,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
                        $this->queryLogger->debug( "SQL ERROR (ignored): $error\n" );
                } else {
                        $sql1line = mb_substr( str_replace( "\n", "\\n", $sql ), 0, 5 * 1024 );
-                       wfLogDBError(
+                       $this->queryLogger->error(
                                "{fname}\t{db_server}\t{errno}\t{error}\t{sql1line}",
                                $this->getLogContext( [
                                        'method' => __METHOD__,
@@ -2805,7 +2808,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
                                                $this->clearFlag( DBO_TRX ); // restore auto-commit
                                        }
                                } catch ( Exception $ex ) {
-                                       MWExceptionHandler::logException( $ex );
+                                       call_user_func( $this->errorLogger, $ex );
                                        $e = $e ?: $ex;
                                        // Some callbacks may use startAtomic/endAtomic, so make sure
                                        // their transactions are ended so other callbacks don't fail
@@ -2839,7 +2842,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
                                        list( $phpCallback ) = $callback;
                                        call_user_func( $phpCallback );
                                } catch ( Exception $ex ) {
-                                       MWExceptionHandler::logException( $ex );
+                                       call_user_func( $this->errorLogger, $ex );
                                        $e = $e ?: $ex;
                                }
                        }
@@ -2872,7 +2875,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
                                list( $phpCallback ) = $callback;
                                $phpCallback( $trigger, $this );
                        } catch ( Exception $ex ) {
-                               MWExceptionHandler::logException( $ex );
+                               call_user_func( $this->errorLogger, $ex );
                                $e = $e ?: $ex;
                        }
                }
@@ -2936,15 +2939,13 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
                        } else {
                                // @TODO: make this an exception at some point
                                $msg = "$fname: Implicit transaction already active (from {$this->mTrxFname}).";
-                               wfLogDBError( $msg );
-                               wfWarn( $msg );
+                               $this->queryLogger->error( $msg );
                                return; // join the main transaction set
                        }
                } elseif ( $this->getFlag( DBO_TRX ) && $mode !== self::TRANSACTION_INTERNAL ) {
                        // @TODO: make this an exception at some point
                        $msg = "$fname: Implicit transaction expected (DBO_TRX set).";
-                       wfLogDBError( $msg );
-                       wfWarn( $msg );
+                       $this->queryLogger->error( $msg );
                        return; // let any writes be in the main transaction
                }
 
@@ -3003,13 +3004,12 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
                        }
                } else {
                        if ( !$this->mTrxLevel ) {
-                               wfWarn( "$fname: No transaction to commit, something got out of sync." );
+                               $this->queryLogger->error( "$fname: No transaction to commit, something got out of sync." );
                                return; // nothing to do
                        } elseif ( $this->mTrxAutomatic ) {
                                // @TODO: make this an exception at some point
                                $msg = "$fname: Explicit commit of implicit transaction.";
-                               wfLogDBError( $msg );
-                               wfWarn( $msg );
+                               $this->queryLogger->error( $msg );
                                return; // wait for the main transaction set commit round
                        }
                }
@@ -3050,7 +3050,8 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
                        }
                } else {
                        if ( !$this->mTrxLevel ) {
-                               wfWarn( "$fname: No transaction to rollback, something got out of sync." );
+                               $this->queryLogger->error(
+                                       "$fname: No transaction to rollback, something got out of sync." );
                                return; // nothing to do
                        } elseif ( $this->getFlag( DBO_TRX ) ) {
                                throw new DBUnexpectedError(
@@ -3119,7 +3120,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
         * @param string $newName Name of table to be created
         * @param bool $temporary Whether the new table should be temporary
         * @param string $fname Calling function name
-        * @throws MWException
+        * @throws RuntimeException
         * @return bool True if operation was successful
         */
        public function duplicateTableStructure( $oldName, $newName, $temporary = false,
@@ -3148,7 +3149,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
         *
         * @param string $prefix Only show VIEWs with this prefix, eg. unit_test_
         * @param string $fname Name of calling function
-        * @throws MWException
+        * @throws RuntimeException
         * @return array
         * @since 1.22
         */
@@ -3160,7 +3161,7 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
         * Differentiates between a TABLE and a VIEW
         *
         * @param string $name Name of the database-structure to test.
-        * @throws MWException
+        * @throws RuntimeException
         * @return bool
         * @since 1.22
         */
@@ -3349,8 +3350,8 @@ abstract class DatabaseBase implements IDatabase, LoggerAwareInterface {
         *   generated dynamically using $filename
         * @param bool|callable $inputCallback Optional function called for each
         *   complete line sent
-        * @throws Exception|MWException
         * @return bool|string
+        * @throws Exception
         */
        public function sourceFile(
                $filename, $lineCallback = false, $resultCallback = false, $fname = false, $inputCallback = false

diff --git a/includes/db/DatabaseMssql.php b/includes/db/DatabaseMssql.php
index 2439c60..f39f491 100644 (file)
--- a/includes/db/DatabaseMssql.php
+++ b/includes/db/DatabaseMssql.php
@@ -777,7 +777,6 @@ class DatabaseMssql extends Database {
         * @return bool
         * @throws DBUnexpectedError
         * @throws Exception
-        * @throws MWException
         */
        function update( $table, $values, $conds, $fname = __METHOD__, $options = [] ) {
                $table = $this->tableName( $table );
@@ -814,7 +813,7 @@ class DatabaseMssql extends Database {
         * @param array $binaryColumns Contains a list of column names that are binary types
         *      This is a custom parameter only present for MS SQL.
         *
-        * @throws MWException|DBUnexpectedError
+        * @throws DBUnexpectedError
         * @return string
         */
        public function makeList( $a, $mode = LIST_COMMA, $binaryColumns = [] ) {
@@ -1074,7 +1073,7 @@ class DatabaseMssql extends Database {
         * Throws an exception if it is invalid.
         * Reference: http://msdn.microsoft.com/en-us/library/aa224033%28v=SQL.80%29.aspx
         * @param string $identifier
-        * @throws MWException
+        * @throws InvalidArgumentException
         * @return string
         */
        private function escapeIdentifier( $identifier ) {

diff --git a/includes/db/DatabaseSqlite.php b/includes/db/DatabaseSqlite.php
index 6bf48e2..6663ec5 100644 (file)
--- a/includes/db/DatabaseSqlite.php
+++ b/includes/db/DatabaseSqlite.php
@@ -944,12 +944,12 @@ class DatabaseSqlite extends Database {
        }
 
        /**
-        * @throws MWException
         * @param string $oldName
         * @param string $newName
         * @param bool $temporary
         * @param string $fname
         * @return bool|ResultWrapper
+        * @throws RuntimeException
         */
        function duplicateTableStructure( $oldName, $newName, $temporary = false, $fname = __METHOD__ ) {
                $res = $this->query( "SELECT sql FROM sqlite_master WHERE tbl_name=" .

diff --git a/includes/db/loadbalancer/LBFactoryMulti.php b/includes/db/loadbalancer/LBFactoryMulti.php
index dd7737b..bbee3b8 100644 (file)
--- a/includes/db/loadbalancer/LBFactoryMulti.php
+++ b/includes/db/loadbalancer/LBFactoryMulti.php
@@ -164,7 +164,7 @@ class LBFactoryMulti extends LBFactory {
 
        /**
         * @param array $conf
-        * @throws MWException
+        * @throws InvalidArgumentException
         */
        public function __construct( array $conf ) {
                parent::__construct( $conf );
@@ -264,7 +264,7 @@ class LBFactoryMulti extends LBFactory {
        /**
         * @param string $cluster
         * @param bool|string $wiki
-        * @throws MWException
+        * @throws InvalidArgumentException
         * @return LoadBalancer
         */
        protected function newExternalLB( $cluster, $wiki = false ) {

diff --git a/includes/db/loadbalancer/LBFactorySimple.php b/includes/db/loadbalancer/LBFactorySimple.php
index d8590b7..908453c 100644 (file)
--- a/includes/db/loadbalancer/LBFactorySimple.php
+++ b/includes/db/loadbalancer/LBFactorySimple.php
@@ -102,10 +102,10 @@ class LBFactorySimple extends LBFactory {
        }
 
        /**
-        * @throws MWException
         * @param string $cluster
         * @param bool|string $wiki
         * @return LoadBalancer
+        * @throws InvalidArgumentException
         */
        protected function newExternalLB( $cluster, $wiki = false ) {
                global $wgExternalServers;

diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
index 6f136df..903c160 100644 (file)
--- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
@@ -776,7 +776,7 @@ class LoadBalancer implements ILoadBalancer {
         * @param bool $dbNameOverride
         * @return IDatabase
         * @throws DBAccessError
-        * @throws MWException
+        * @throws InvalidArgumentException
         */
        protected function reallyOpenConnection( $server, $dbNameOverride = false ) {
                if ( $this->disabled ) {