From 359b1b7cbe852640aed7d3c95d682bcb6e9afaa2 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Wed, 24 Jun 2015 17:24:00 -0700
Subject: [PATCH] database: Small DB class cleanups

* Moved some duplicated logic to assertOpen() method
* Override doc type for mConn for the mysqli subclass
* Fixed a few code comments

Change-Id: I78d595554ed51f64ca7cf7bd7ce369a492a59145
---
 includes/db/Database.php          | 27 +++++++++++++++------------
 includes/db/DatabaseMysql.php     |  3 +++
 includes/db/DatabaseMysqlBase.php | 14 ++++----------
 includes/db/DatabaseMysqli.php    |  3 +++
 4 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/includes/db/Database.php b/includes/db/Database.php
index 94cf1f2c3e..032e926d15 100644
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -1019,6 +1019,17 @@ abstract class DatabaseBase implements IDatabase {
 		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
@@ -1175,9 +1186,7 @@ abstract class DatabaseBase implements IDatabase {
 		$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 );
@@ -3648,9 +3657,7 @@ abstract class DatabaseBase implements IDatabase {
 		}
 
 		# 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 );
@@ -3715,9 +3722,7 @@ abstract class DatabaseBase implements IDatabase {
 		}
 
 		# 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();
@@ -3774,9 +3779,7 @@ abstract class DatabaseBase implements IDatabase {
 		}
 
 		# 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
diff --git a/includes/db/DatabaseMysql.php b/includes/db/DatabaseMysql.php
index 823d9b67a6..915375351f 100644
--- a/includes/db/DatabaseMysql.php
+++ b/includes/db/DatabaseMysql.php
@@ -73,6 +73,9 @@ class DatabaseMysql extends DatabaseMysqlBase {
 
 		$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 );
diff --git a/includes/db/DatabaseMysqlBase.php b/includes/db/DatabaseMysqlBase.php
index a18964803f..9285d70d3b 100644
--- a/includes/db/DatabaseMysqlBase.php
+++ b/includes/db/DatabaseMysqlBase.php
@@ -59,22 +59,16 @@ abstract class DatabaseMysqlBase extends DatabaseBase {
 	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 );
diff --git a/includes/db/DatabaseMysqli.php b/includes/db/DatabaseMysqli.php
index d2b5ecb1cf..d4106bec1b 100644
--- a/includes/db/DatabaseMysqli.php
+++ b/includes/db/DatabaseMysqli.php
@@ -29,6 +29,9 @@
  * @see Database
  */
 class DatabaseMysqli extends DatabaseMysqlBase {
+	/** @var mysqli */
+	protected $mConn;
+
 	/**
 	 * @param string $sql
 	 * @return resource
-- 
2.20.1