From: Aaron Schulz <aschulz@wikimedia.org>
Date: Wed, 22 Aug 2018 01:34:51 +0000 (-0700)
Subject: rdbms: make Database::open() protected
X-Git-Tag: 1.34.0-rc.0~4342^2
X-Git-Url: https://git.cyclocoop.org/%27.WWW_URL.%27admin//%27%241/%27?a=commitdiff_plain;h=41a37d14faf91d17ac193f135fbf4c8b41529ce9;p=lhc%2Fweb%2Fwiklou.git

rdbms: make Database::open() protected

This is not called externally and there is little reason for that to
change. The current caller pattern is to use factory(), possibly with
initConnection() afterwards, or to use a LoadBalancer to begin with.

Change-Id: Ib1fdd5c960f1ed877fcd17bcb99b999d5d894716
---

diff --git a/includes/db/DatabaseOracle.php b/includes/db/DatabaseOracle.php
index 49777627af..876b9bb9fa 100644
--- a/includes/db/DatabaseOracle.php
+++ b/includes/db/DatabaseOracle.php
@@ -81,15 +81,6 @@ class DatabaseOracle extends Database {
 		return false;
 	}
 
-	/**
-	 * Usually aborts on failure
-	 * @param string $server
-	 * @param string $user
-	 * @param string $password
-	 * @param string $dbName
-	 * @throws DBConnectionError
-	 * @return resource|null
-	 */
 	function open( $server, $user, $password, $dbName ) {
 		global $wgDBOracleDRCP;
 		if ( !function_exists( 'oci_connect' ) ) {
@@ -173,7 +164,7 @@ class DatabaseOracle extends Database {
 		$this->doQuery( 'ALTER SESSION SET NLS_TIMESTAMP_TZ_FORMAT=\'DD-MM-YYYY HH24:MI:SS.FF6\'' );
 		$this->doQuery( 'ALTER SESSION SET NLS_NUMERIC_CHARACTERS=\'.,\'' );
 
-		return $this->conn;
+		return (bool)$this->conn;
 	}
 
 	/**
diff --git a/includes/libs/rdbms/database/DBConnRef.php b/includes/libs/rdbms/database/DBConnRef.php
index eba1657f29..0de90c9d04 100644
--- a/includes/libs/rdbms/database/DBConnRef.php
+++ b/includes/libs/rdbms/database/DBConnRef.php
@@ -178,10 +178,6 @@ class DBConnRef implements IDatabase {
 		return $this->__call( __FUNCTION__, func_get_args() );
 	}
 
-	public function open( $server, $user, $password, $dbName ) {
-		return $this->__call( __FUNCTION__, func_get_args() );
-	}
-
 	public function fetchObject( $res ) {
 		return $this->__call( __FUNCTION__, func_get_args() );
 	}
diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index e35e0827b0..88b6d2079f 100644
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -373,6 +373,18 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 		}
 	}
 
+	/**
+	 * Open a new connection to the database (closing any existing one)
+	 *
+	 * @param string $server Database server host
+	 * @param string $user Database user name
+	 * @param string $password Database user password
+	 * @param string $dbName Database name
+	 * @return bool
+	 * @throws DBConnectionError
+	 */
+	abstract protected function open( $server, $user, $password, $dbName );
+
 	/**
 	 * Construct a Database subclass instance given a database type and parameters
 	 *
diff --git a/includes/libs/rdbms/database/DatabaseMssql.php b/includes/libs/rdbms/database/DatabaseMssql.php
index fed6f14616..8d32b12f63 100644
--- a/includes/libs/rdbms/database/DatabaseMssql.php
+++ b/includes/libs/rdbms/database/DatabaseMssql.php
@@ -77,16 +77,7 @@ class DatabaseMssql extends Database {
 		parent::__construct( $params );
 	}
 
-	/**
-	 * Usually aborts on failure
-	 * @param string $server
-	 * @param string $user
-	 * @param string $password
-	 * @param string $dbName
-	 * @throws DBConnectionError
-	 * @return bool|resource|null
-	 */
-	public function open( $server, $user, $password, $dbName ) {
+	protected function open( $server, $user, $password, $dbName ) {
 		# Test for driver support, to avoid suppressed fatal error
 		if ( !function_exists( 'sqlsrv_connect' ) ) {
 			throw new DBConnectionError(
@@ -130,7 +121,7 @@ class DatabaseMssql extends Database {
 
 		$this->opened = true;
 
-		return $this->conn;
+		return (bool)$this->conn;
 	}
 
 	/**
diff --git a/includes/libs/rdbms/database/DatabaseMysqlBase.php b/includes/libs/rdbms/database/DatabaseMysqlBase.php
index 57fab54936..0f575516b4 100644
--- a/includes/libs/rdbms/database/DatabaseMysqlBase.php
+++ b/includes/libs/rdbms/database/DatabaseMysqlBase.php
@@ -120,15 +120,7 @@ abstract class DatabaseMysqlBase extends Database {
 		return 'mysql';
 	}
 
-	/**
-	 * @param string $server
-	 * @param string $user
-	 * @param string $password
-	 * @param string $dbName
-	 * @throws Exception|DBConnectionError
-	 * @return bool
-	 */
-	public function open( $server, $user, $password, $dbName ) {
+	protected function open( $server, $user, $password, $dbName ) {
 		# Close/unset connection handle
 		$this->close();
 
diff --git a/includes/libs/rdbms/database/DatabasePostgres.php b/includes/libs/rdbms/database/DatabasePostgres.php
index a959d72ba8..3c2f145656 100644
--- a/includes/libs/rdbms/database/DatabasePostgres.php
+++ b/includes/libs/rdbms/database/DatabasePostgres.php
@@ -86,7 +86,7 @@ class DatabasePostgres extends Database {
 		return false;
 	}
 
-	public function open( $server, $user, $password, $dbName ) {
+	protected function open( $server, $user, $password, $dbName ) {
 		# Test for Postgres support, to avoid suppressed fatal error
 		if ( !function_exists( 'pg_connect' ) ) {
 			throw new DBConnectionError(
diff --git a/includes/libs/rdbms/database/DatabaseSqlite.php b/includes/libs/rdbms/database/DatabaseSqlite.php
index 25fbba09e5..1b9675add6 100644
--- a/includes/libs/rdbms/database/DatabaseSqlite.php
+++ b/includes/libs/rdbms/database/DatabaseSqlite.php
@@ -155,24 +155,14 @@ class DatabaseSqlite extends Database {
 		return false;
 	}
 
-	/** Open an SQLite database and return a resource handle to it
-	 *  NOTE: only $dbName is used, the other parameters are irrelevant for SQLite databases
-	 *
-	 * @param string $server
-	 * @param string $user Unused
-	 * @param string $pass
-	 * @param string $dbName
-	 *
-	 * @throws DBConnectionError
-	 * @return bool
-	 */
-	function open( $server, $user, $pass, $dbName ) {
+	protected function open( $server, $user, $pass, $dbName ) {
 		$this->close();
 		$fileName = self::generateFileName( $this->dbDir, $dbName );
 		if ( !is_readable( $fileName ) ) {
 			$this->conn = false;
 			throw new DBConnectionError( $this, "SQLite database not accessible" );
 		}
+		// Only $dbName is used, the other parameters are irrelevant for SQLite databases
 		$this->openFile( $fileName, $dbName );
 
 		return (bool)$this->conn;
diff --git a/includes/libs/rdbms/database/IDatabase.php b/includes/libs/rdbms/database/IDatabase.php
index 7da259d9f0..f97db3a7ca 100644
--- a/includes/libs/rdbms/database/IDatabase.php
+++ b/includes/libs/rdbms/database/IDatabase.php
@@ -370,18 +370,6 @@ interface IDatabase {
 	 */
 	public function getType();
 
-	/**
-	 * Open a new connection to the database (closing any existing one)
-	 *
-	 * @param string $server Database server host
-	 * @param string $user Database user name
-	 * @param string $password Database user password
-	 * @param string $dbName Database name
-	 * @return bool
-	 * @throws DBConnectionError
-	 */
-	public function open( $server, $user, $password, $dbName );
-
 	/**
 	 * Fetch the next row from the given result object, in object form.
 	 * Fields can be retrieved with $row->fieldname, with fields acting like