Merge "database: Small DB class cleanups"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Sun, 28 Jun 2015 00:26:07 +0000 (00:26 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Sun, 28 Jun 2015 00:26:07 +0000 (00:26 +0000)
1  2 
includes/db/Database.php
includes/db/DatabaseMysql.php
includes/db/DatabaseMysqlBase.php
includes/db/DatabaseMysqli.php

diff --combined includes/db/Database.php
@@@ -1019,6 -1019,17 +1019,17 @@@ abstract class DatabaseBase implements 
                return $closed;
        }
  
+       /**
+        * Make sure isOpen() returns true as a sanity check
+        *
+        * @throws DBUnexpectedError
+        */
+       protected function assertOpen() {
+               if ( !$this->isOpen() ) {
+                       throw new DBUnexpectedError( $this, "DB connection was already closed." );
+               }
+       }
        /**
         * Closes underlying database connection
         * @since 1.20
                $queryId = MWDebug::query( $sql, $fname, $isMaster );
  
                # Avoid fatals if close() was called
-               if ( !$this->isOpen() ) {
-                       throw new DBUnexpectedError( $this, "DB connection was already closed." );
-               }
+               $this->assertOpen();
  
                # Do the query and handle errors
                $startTime = microtime( true );
                }
  
                # Avoid fatals if close() was called
-               if ( !$this->isOpen() ) {
-                       throw new DBUnexpectedError( $this, "DB connection was already closed." );
-               }
+               $this->assertOpen();
  
                $this->doBegin( $fname );
                $this->mTrxTimestamp = microtime( true );
                }
  
                # Avoid fatals if close() was called
-               if ( !$this->isOpen() ) {
-                       throw new DBUnexpectedError( $this, "DB connection was already closed." );
-               }
+               $this->assertOpen();
  
                $this->runOnTransactionPreCommitCallbacks();
                $writeTime = $this->pendingWriteQueryDuration();
                }
  
                # Avoid fatals if close() was called
-               if ( !$this->isOpen() ) {
-                       throw new DBUnexpectedError( $this, "DB connection was already closed." );
-               }
+               $this->assertOpen();
  
                $this->doRollback( $fname );
                $this->mTrxIdleCallbacks = array(); // cancel
         * @param string $prefix Only show tables with this prefix, e.g. mw_
         * @param string $fname Calling function name
         * @throws MWException
 +       * @return array
         */
        function listTables( $prefix = null, $fname = __METHOD__ ) {
                throw new MWException( 'DatabaseBase::listTables is not implemented in descendant class' );
         * @param string $prefix Only show VIEWs with this prefix, eg. unit_test_
         * @param string $fname Name of calling function
         * @throws MWException
 +       * @return array
         * @since 1.22
         */
        public function listViews( $prefix = null, $fname = __METHOD__ ) {
         *
         * @param string $name Name of the database-structure to test.
         * @throws MWException
 +       * @return bool
         * @since 1.22
         */
        public function isView( $name ) {
@@@ -33,12 -33,10 +33,12 @@@ class DatabaseMysql extends DatabaseMys
         * @return resource False on error
         */
        protected function doQuery( $sql ) {
 +              $conn = $this->getBindingHandle();
 +
                if ( $this->bufferResults() ) {
 -                      $ret = mysql_query( $sql, $this->mConn );
 +                      $ret = mysql_query( $sql, $conn );
                } else {
 -                      $ret = mysql_unbuffered_query( $sql, $this->mConn );
 +                      $ret = mysql_unbuffered_query( $sql, $conn );
                }
  
                return $ret;
@@@ -50,7 -48,8 +50,7 @@@
         * @throws DBConnectionError
         */
        protected function mysqlConnect( $realServer ) {
 -              # Fail now
 -              # Otherwise we get a suppressed fatal error, which is very hard to track down
 +              # Avoid a suppressed fatal error, which is very hard to track down
                if ( !extension_loaded( 'mysql' ) ) {
                        throw new DBConnectionError(
                                $this,
@@@ -74,6 -73,9 +74,9 @@@
  
                $conn = false;
  
+               # The kernel's default SYN retransmission period is far too slow for us,
+               # so we use a short timeout plus a manual retry. Retrying means that a small
+               # but finite rate of SYN packet loss won't cause user-visible errors.
                for ( $i = 0; $i < $numAttempts && !$conn; $i++ ) {
                        if ( $i > 1 ) {
                                usleep( 1000 );
         * @return bool
         */
        protected function mysqlSetCharset( $charset ) {
 +              $conn = $this->getBindingHandle();
 +
                if ( function_exists( 'mysql_set_charset' ) ) {
 -                      return mysql_set_charset( $charset, $this->mConn );
 +                      return mysql_set_charset( $charset, $conn );
                } else {
                        return $this->query( 'SET NAMES ' . $charset, __METHOD__ );
                }
         * @return bool
         */
        protected function closeConnection() {
 -              return mysql_close( $this->mConn );
 +              $conn = $this->getBindingHandle();
 +
 +              return mysql_close( $conn );
        }
  
        /**
         * @return int
         */
        function insertId() {
 -              return mysql_insert_id( $this->mConn );
 +              $conn = $this->getBindingHandle();
 +
 +              return mysql_insert_id( $conn );
        }
  
        /**
         * @return int
         */
        function affectedRows() {
 -              return mysql_affected_rows( $this->mConn );
 +              $conn = $this->getBindingHandle();
 +
 +              return mysql_affected_rows( $conn );
        }
  
        /**
         * @return bool
         */
        function selectDB( $db ) {
 +              $conn = $this->getBindingHandle();
 +
                $this->mDBname = $db;
  
 -              return mysql_select_db( $db, $this->mConn );
 +              return mysql_select_db( $db, $conn );
        }
  
        protected function mysqlFreeResult( $res ) {
        }
  
        protected function mysqlRealEscapeString( $s ) {
 -              return mysql_real_escape_string( $s, $this->mConn );
 +              $conn = $this->getBindingHandle();
 +
 +              return mysql_real_escape_string( $s, $conn );
        }
  
        protected function mysqlPing() {
 -              return mysql_ping( $this->mConn );
 +              $conn = $this->getBindingHandle();
 +
 +              return mysql_ping( $conn );
        }
  }
@@@ -59,22 -59,16 +59,16 @@@ abstract class DatabaseMysqlBase extend
        function open( $server, $user, $password, $dbName ) {
                global $wgAllDBsAreLocalhost, $wgSQLMode;
  
-               # Debugging hack -- fake cluster
-               if ( $wgAllDBsAreLocalhost ) {
-                       $realServer = 'localhost';
-               } else {
-                       $realServer = $server;
-               }
+               # Close/unset connection handle
                $this->close();
+               # Debugging hack -- fake cluster
+               $realServer = $wgAllDBsAreLocalhost ? 'localhost' : $server;
                $this->mServer = $server;
                $this->mUser = $user;
                $this->mPassword = $password;
                $this->mDBname = $dbName;
  
-               # The kernel's default SYN retransmission period is far too slow for us,
-               # so we use a short timeout plus a manual retry. Retrying means that a small
-               # but finite rate of SYN packet loss won't cause user-visible errors.
-               $this->mConn = false;
                $this->installErrorHandler();
                try {
                        $this->mConn = $this->mysqlConnect( $realServer );
        function ping() {
                $ping = $this->mysqlPing();
                if ( $ping ) {
 +                      // Connection was good or lost but reconnected...
 +                      // @note: mysqlnd (php 5.6+) does not support this (PHP bug 52561)
                        return true;
                }
  
 +              // Try a full disconnect/reconnect cycle if ping() failed
                $this->closeConnection();
                $this->mOpened = false;
                $this->mConn = false;
                        ( $this->lastErrno() == 1290 && strpos( $this->lastError(), '--read-only' ) !== false );
        }
  
 +      /**
 +       * Get the underlying binding handle, mConn
 +       *
 +       * Makes sure that mConn is set (disconnects and ping() failure can unset it).
 +       * This catches broken callers than catch and ignore disconnection exceptions.
 +       * Unlike checking isOpen(), this is safe to call inside of open().
 +       *
 +       * @return resource|object
 +       * @throws DBUnexpectedError
 +       * @since 1.26
 +       */
 +      protected function getBindingHandle() {
 +              if ( !$this->mConn ) {
 +                      throw new DBUnexpectedError(
 +                              $this,
 +                              'DB connection was already closed or the connection dropped.'
 +                      );
 +              }
 +
 +              return $this->mConn;
 +      }
 +
        /**
         * @param string $oldName
         * @param string $newName
   * @see Database
   */
  class DatabaseMysqli extends DatabaseMysqlBase {
+       /** @var mysqli */
+       protected $mConn;
        /**
         * @param string $sql
         * @return resource
         */
        protected function doQuery( $sql ) {
 +              $conn = $this->getBindingHandle();
 +
                if ( $this->bufferResults() ) {
 -                      $ret = $this->mConn->query( $sql );
 +                      $ret = $conn->query( $sql );
                } else {
 -                      $ret = $this->mConn->query( $sql, MYSQLI_USE_RESULT );
 +                      $ret = $conn->query( $sql, MYSQLI_USE_RESULT );
                }
  
                return $ret;
@@@ -52,8 -53,8 +55,8 @@@
         */
        protected function mysqlConnect( $realServer ) {
                global $wgDBmysql5;
 -              # Fail now
 -              # Otherwise we get a suppressed fatal error, which is very hard to track down
 +
 +              # Avoid suppressed fatal error, which is very hard to track down
                if ( !function_exists( 'mysqli_init' ) ) {
                        throw new DBConnectionError( $this, "MySQLi functions missing,"
                                . " have you compiled PHP with the --with-mysqli option?\n" );
         * @return bool
         */
        protected function mysqlSetCharset( $charset ) {
 -              if ( method_exists( $this->mConn, 'set_charset' ) ) {
 -                      return $this->mConn->set_charset( $charset );
 +              $conn = $this->getBindingHandle();
 +
 +              if ( method_exists( $conn, 'set_charset' ) ) {
 +                      return $conn->set_charset( $charset );
                } else {
                        return $this->query( 'SET NAMES ' . $charset, __METHOD__ );
                }
         * @return bool
         */
        protected function closeConnection() {
 -              return $this->mConn->close();
 +              $conn = $this->getBindingHandle();
 +
 +              return $conn->close();
        }
  
        /**
         * @return int
         */
        function insertId() {
 -              return (int)$this->mConn->insert_id;
 +              $conn = $this->getBindingHandle();
 +
 +              return (int)$conn->insert_id;
        }
  
        /**
         * @return int
         */
        function affectedRows() {
 -              return $this->mConn->affected_rows;
 +              $conn = $this->getBindingHandle();
 +
 +              return $conn->affected_rows;
        }
  
        /**
         * @return bool
         */
        function selectDB( $db ) {
 +              $conn = $this->getBindingHandle();
 +
                $this->mDBname = $db;
  
 -              return $this->mConn->select_db( $db );
 +              return $conn->select_db( $db );
        }
  
        /**
         * @return string
         */
        protected function mysqlRealEscapeString( $s ) {
 -              return $this->mConn->real_escape_string( $s );
 +              $conn = $this->getBindingHandle();
 +
 +              return $conn->real_escape_string( $s );
        }
  
        protected function mysqlPing() {
 -              return $this->mConn->ping();
 +              $conn = $this->getBindingHandle();
 +
 +              return $conn->ping();
        }
  
        /**