Database class parameter and documentation cleanups
authorAaron Schulz <aschulz@wikimedia.org>
Mon, 19 Sep 2016 23:34:32 +0000 (16:34 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Tue, 20 Sep 2016 06:23:30 +0000 (23:23 -0700)
* Document various parameter arrays.
* Fix $user = false loophole in Database::__construct().
* Set the Postgres port *before* calling super, as it is
  needed by open().
* Remove 'chronProt' parameter as it is lazy-loaded.

Change-Id: Icc1037efa1eee7ae6fdd2919f60001e6e29ae55c

includes/libs/rdbms/database/Database.php
includes/libs/rdbms/database/DatabasePostgres.php
includes/libs/rdbms/database/DatabaseSqlite.php
includes/libs/rdbms/lbfactory/LBFactory.php
includes/libs/rdbms/lbfactory/LBFactoryMulti.php
includes/libs/rdbms/lbfactory/LBFactorySimple.php
includes/libs/rdbms/loadbalancer/ILoadBalancer.php

index f9e9296..a5b9284 100644 (file)
@@ -231,16 +231,12 @@ abstract class Database implements IDatabase, LoggerAwareInterface {
        protected $trxProfiler;
 
        /**
-        * Constructor.
-        *
-        * FIXME: It is possible to construct a Database object with no associated
-        * connection object, by specifying no parameters to __construct(). This
-        * feature is deprecated and should be removed.
+        * Constructor and database handle and attempt to connect to the DB server
         *
         * IDatabase classes should not be constructed directly in external
-        * code. DatabaseBase::factory() should be used instead.
+        * code. Database::factory() should be used instead.
         *
-        * @param array $params Parameters passed from DatabaseBase::factory()
+        * @param array $params Parameters passed from Database::factory()
         */
        function __construct( array $params ) {
                $server = $params['host'];
@@ -287,37 +283,59 @@ abstract class Database implements IDatabase, LoggerAwareInterface {
 
                if ( $user ) {
                        $this->open( $server, $user, $password, $dbName );
+               } elseif ( $this->requiresDatabaseUser() ) {
+                       throw new InvalidArgumentException( "No database user provided." );
                }
 
+               // Set the domain object after open() sets the relevant fields
                $this->currentDomain = ( $this->mDBname != '' )
                        ? new DatabaseDomain( $this->mDBname, null, $this->mTablePrefix )
                        : DatabaseDomain::newUnspecified();
        }
 
        /**
-        * Given a DB type, construct the name of the appropriate child class of
-        * IDatabase. This is designed to replace all of the manual stuff like:
-        *    $class = 'Database' . ucfirst( strtolower( $dbType ) );
-        * as well as validate against the canonical list of DB types we have
-        *
-        * This factory function is mostly useful for when you need to connect to a
-        * database other than the MediaWiki default (such as for external auth,
-        * an extension, et cetera). Do not use this to connect to the MediaWiki
-        * database. Example uses in core:
-        * @see LoadBalancer::reallyOpenConnection()
-        * @see ForeignDBRepo::getMasterDB()
-        * @see WebInstallerDBConnect::execute()
-        *
-        * @since 1.18
-        *
-        * @param string $dbType A possible DB type
-        * @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
+        * Construct a Database subclass instance given a database type and parameters
+        *
+        * This also connects to the database immediately upon object construction
+        *
+        * @param string $dbType A possible DB type (sqlite, mysql, postgres)
+        * @param array $p Parameter map with keys:
+        *   - host : The hostname of the DB server
+        *   - user : The name of the database user the client operates under
+        *   - password : The password for the database user
+        *   - dbname : The name of the database to use where queries do not specify one.
+        *      The database must exist or an error might be thrown. Setting this to the empty string
+        *      will avoid any such errors and make the handle have no implicit database scope. This is
+        *      useful for queries like SHOW STATUS, CREATE DATABASE, or DROP DATABASE. Note that a
+        *      "database" in Postgres is rougly equivalent to an entire MySQL server. This the domain
+        *      in which user names and such are defined, e.g. users are database-specific in Postgres.
+        *   - schema : The database schema to use (if supported). A "schema" in Postgres is roughly
+        *      equivalent to a "database" in MySQL. Note that MySQL and SQLite do not use schemas.
+        *   - tablePrefix : Optional table prefix that is implicitly added on to all table names
+        *      recognized in queries. This can be used in place of schemas for handle site farms.
+        *   - flags : Optional bitfield of DBO_* constants that define connection, protocol,
+        *      buffering, and transaction behavior. It is STRONGLY adviced to leave the DBO_DEFAULT
+        *      flag in place UNLESS this this database simply acts as a key/value store.
+        *   - driver: Optional name of a specific DB client driver. For MySQL, there is the old
+        *      'mysql' driver and the newer 'mysqli' driver.
+        *   - variables: Optional map of session variables to set after connecting. This can be
+        *      used to adjust lock timeouts or encoding modes and the like.
+        *   - connLogger: Optional PSR-3 logger interface instance.
+        *   - queryLogger: Optional PSR-3 logger interface instance.
+        *   - profiler: Optional class name or object with profileIn()/profileOut() methods.
+        *      These will be called in query(), using a simplified version of the SQL that also
+        *      includes the agent as a SQL comment.
+        *   - trxProfiler: Optional TransactionProfiler instance.
+        *   - errorLogger: Optional callback that takes an Exception and logs it.
+        *   - cliMode: Whether to consider the execution context that of a CLI script.
+        *   - agent: Optional name used to identify the end-user in query profiling/logging.
+        *   - srvCache: Optional BagOStuff instance to an APC-style cache.
+        * @return Database|null If the database driver or extension cannot be found
         * @throws InvalidArgumentException If the database driver or extension cannot be found
+        * @since 1.18
         */
        final public static function factory( $dbType, $p = [] ) {
-               $canonicalDBTypes = [
+               static $canonicalDBTypes = [
                        'mysql' => [ 'mysqli', 'mysql' ],
                        'postgres' => [],
                        'sqlite' => [],
@@ -667,7 +685,7 @@ abstract class Database implements IDatabase, LoggerAwareInterface {
        }
 
        /**
-        * Create a log context to pass to PSR logging functions.
+        * Create a log context to pass to PSR-3 logger functions.
         *
         * @param array $extras Additional data to add to context
         * @return array
@@ -3456,6 +3474,14 @@ abstract class Database implements IDatabase, LoggerAwareInterface {
                $this->tableAliases = $aliases;
        }
 
+       /**
+        * @return bool Whether a DB user is required to access the DB
+        * @since 1.28
+        */
+       protected function requiresDatabaseUser() {
+               return true;
+       }
+
        /**
         * @since 1.19
         * @return string
index b07ac16..e79b28b 100644 (file)
@@ -43,8 +43,8 @@ class DatabasePostgres extends DatabaseBase {
        private $mCoreSchema;
 
        public function __construct( array $params ) {
-               parent::__construct( $params );
                $this->port = isset( $params['port'] ) ? $params['port'] : false;
+               parent::__construct( $params );
        }
 
        function getType() {
index 8881983..6614898 100644 (file)
@@ -1046,11 +1046,14 @@ class DatabaseSqlite extends DatabaseBase {
                return $this->query( $sql, $fName );
        }
 
+       protected function requiresDatabaseUser() {
+               return false; // just a file
+       }
+
        /**
         * @return string
         */
        public function __toString() {
                return 'SQLite ' . (string)$this->mConn->getAttribute( PDO::ATTR_SERVER_VERSION );
        }
-
 }
index 40ba458..38132c7 100644 (file)
@@ -80,8 +80,26 @@ abstract class LBFactory {
                [ 'replLogger', 'connLogger', 'queryLogger', 'perfLogger' ];
 
        /**
-        * @TODO: document base params here
-        * @param array $conf
+        * Construct a manager of ILoadBalancer objects
+        *
+        * Sub-classes will extend the required keys in $conf with additional parameters
+        *
+        * @param $conf $params Array with keys:
+        *  - localDomain: A DatabaseDomain or domain ID string.
+        *  - readOnlyReason : Reason the master DB is read-only if so [optional]
+        *  - srvCache : BagOStuff object for server cache [optional]
+        *  - memCache : BagOStuff object for cluster memory cache [optional]
+        *  - wanCache : WANObjectCache object [optional]
+        *  - hostname : The name of the current server [optional]
+        *  - cliMode: Whether the execution context is a CLI script. [optional]
+        *  - profiler : Class name or instance with profileIn()/profileOut() methods. [optional]
+        *  - trxProfiler: TransactionProfiler instance. [optional]
+        *  - replLogger: PSR-3 logger instance. [optional]
+        *  - connLogger: PSR-3 logger instance. [optional]
+        *  - queryLogger: PSR-3 logger instance. [optional]
+        *  - perfLogger: PSR-3 logger instance. [optional]
+        *  - errorLogger : Callback that takes an Exception and logs it. [optional]
+        * @throws InvalidArgumentException
         */
        public function __construct( array $conf ) {
                $this->localDomain = isset( $conf['localDomain'] )
@@ -107,8 +125,6 @@ abstract class LBFactory {
                                trigger_error( E_WARNING, get_class( $e ) . ': ' . $e->getMessage() );
                        };
 
-               $this->chronProt = isset( $conf['chronProt'] ) ? $conf['chronProt'] : null;
-
                $this->profiler = isset( $params['profiler'] ) ? $params['profiler'] : null;
                $this->trxProfiler = isset( $conf['trxProfiler'] )
                        ? $conf['trxProfiler']
index 0f1493a..25e1fe0 100644 (file)
  * A multi-database, multi-master factory for Wikimedia and similar installations.
  * Ignores the old configuration globals.
  *
- * Template override precedence (highest => lowest):
- *   - templateOverridesByServer
- *   - masterTemplateOverrides
- *   - templateOverridesBySection/templateOverridesByCluster
- *   - externalTemplateOverrides
- *   - serverTemplate
- * Overrides only work on top level keys (so nested values will not be merged).
- *
- * Configuration:
- *     sectionsByDB                A map of database names to section names.
- *
- *     sectionLoads                A 2-d map. For each section, gives a map of server names to
- *                                 load ratios. For example:
- *                                 [
- *                                     'section1' => [
- *                                         'db1' => 100,
- *                                         'db2' => 100
- *                                     ]
- *                                 ]
- *
- *     serverTemplate              A server info associative array as documented for $wgDBservers.
- *                                 The host, hostName and load entries will be overridden.
- *
- *     groupLoadsBySection         A 3-d map giving server load ratios for each section and group.
- *                                 For example:
- *                                 [
- *                                     'section1' => [
- *                                         'group1' => [
- *                                             'db1' => 100,
- *                                             'db2' => 100
- *                                         ]
- *                                     ]
- *                                 ]
- *
- *     groupLoadsByDB              A 3-d map giving server load ratios by DB name.
- *
- *     hostsByName                 A map of hostname to IP address.
- *
- *     externalLoads               A map of external storage cluster name to server load map.
- *
- *     externalTemplateOverrides   A set of server info keys overriding serverTemplate for external
- *                                 storage.
- *
- *     templateOverridesByServer   A 2-d map overriding serverTemplate and
- *                                 externalTemplateOverrides on a server-by-server basis. Applies
- *                                 to both core and external storage.
- *     templateOverridesBySection  A 2-d map overriding the server info by section.
- *     templateOverridesByCluster  A 2-d map overriding the server info by external storage cluster.
- *
- *     masterTemplateOverrides     An override array for all master servers.
- *
- *     loadMonitorClass            Name of the LoadMonitor class to always use.
- *
- *     readOnlyBySection           A map of section name to read-only message.
- *                                 Missing or false for read/write.
- *
  * @ingroup Database
  */
 class LBFactoryMulti extends LBFactory {
@@ -141,8 +85,6 @@ class LBFactoryMulti extends LBFactory {
         */
        private $readOnlyBySection = [];
 
-       // Other stuff
-
        /** @var array Load balancer factory configuration */
        private $conf;
 
@@ -162,8 +104,60 @@ class LBFactoryMulti extends LBFactory {
        private $lastSection;
 
        /**
-        * @param array $conf
-        * @throws InvalidArgumentException
+        * @see LBFactory::__construct()
+        *
+        * Template override precedence (highest => lowest):
+        *   - templateOverridesByServer
+        *   - masterTemplateOverrides
+        *   - templateOverridesBySection/templateOverridesByCluster
+        *   - externalTemplateOverrides
+        *   - serverTemplate
+        * Overrides only work on top level keys (so nested values will not be merged).
+        *
+        * Server configuration maps should be of the format Database::factory() requires.
+        * Additionally, a 'max lag' key should also be set on server maps, indicating how stale the
+        * data can be before the load balancer tries to avoid using it. The map can have 'is static'
+        * set to disable blocking  replication sync checks (intended for archive servers with
+        * unchanging data).
+        *
+        * @param array $conf Parameters of LBFactory::__construct() as well as:
+        *   - sectionsByDB                Map of database names to section names.
+        *   - sectionLoads                2-d map. For each section, gives a map of server names to
+        *                                 load ratios. For example:
+        *                                 [
+        *                                     'section1' => [
+        *                                         'db1' => 100,
+        *                                         'db2' => 100
+        *                                     ]
+        *                                 ]
+        *   - serverTemplate              Server configuration map intended for Database::factory().
+        *                                 Note that "host", "hostName" and "load" entries will be
+        *                                 overridden by "sectionLoads" and "hostsByName".
+        *   - groupLoadsBySection         3-d map giving server load ratios for each section/group.
+        *                                 For example:
+        *                                 [
+        *                                     'section1' => [
+        *                                         'group1' => [
+        *                                             'db1' => 100,
+        *                                             'db2' => 100
+        *                                         ]
+        *                                     ]
+        *                                 ]
+        *   - groupLoadsByDB              3-d map giving server load ratios by DB name.
+        *   - hostsByName                 Map of hostname to IP address.
+        *   - externalLoads               Map of external storage cluster name to server load map.
+        *   - externalTemplateOverrides   Set of server configuration maps overriding
+        *                                 "serverTemplate" for external storage.
+        *   - templateOverridesByServer   2-d map overriding "serverTemplate" and
+        *                                 "externalTemplateOverrides" on a server-by-server basis.
+        *                                 Applies to both core and external storage.
+        *   - templateOverridesBySection  2-d map overriding the server configuration maps by section.
+        *   - templateOverridesByCluster  2-d map overriding the server configuration maps by external
+        *                                 storage cluster.
+        *   - masterTemplateOverrides     Server configuration map overrides for all master servers.
+        *   - loadMonitorClass            Name of the LoadMonitor class to always use.
+        *   - readOnlyBySection           A map of section name to read-only message.
+        *                                 Missing or false for read/write.
         */
        public function __construct( array $conf ) {
                parent::__construct( $conf );
index 0949092..b90afe6 100644 (file)
@@ -38,6 +38,18 @@ class LBFactorySimple extends LBFactory {
        /** @var string */
        private $loadMonitorClass;
 
+       /**
+        * @see LBFactory::__construct()
+        * @param array $conf Parameters of LBFactory::__construct() as well as:
+        *   - servers : list of server configuration maps to Database::factory().
+        *      Additionally, the server maps should have a 'load' key, which is used to decide
+        *      how often clients connect to one server verses the others. A 'max lag' key should
+        *      also be set on server maps, indicating how stale the data can be before the load
+        *      balancer tries to avoid using it. The map can have 'is static' set to disable blocking
+        *      replication sync checks (intended for archive servers with unchanging data).
+        *   - externalClusters : map of cluster names to server arrays. The servers arrays have the
+        *      same format as "servers" above.
+        */
        public function __construct( array $conf ) {
                parent::__construct( $conf );
 
index 0f6bea3..cdbfc77 100644 (file)
  */
 interface ILoadBalancer {
        /**
-        * @param array $params Array with keys:
+        * Construct a manager of IDatabase connection objects
+        *
+        * @param array $params Parameter map with keys:
         *  - servers : Required. Array of server info structures.
+        *  - localDomain: A DatabaseDomain or domain ID string.
         *  - loadMonitor : Name of a class used to fetch server lag and load.
         *  - readOnlyReason : Reason the master DB is read-only if so [optional]
         *  - waitTimeout : Maximum time to wait for replicas for consistency [optional]
         *  - srvCache : BagOStuff object for server cache [optional]
         *  - memCache : BagOStuff object for cluster memory cache [optional]
         *  - wanCache : WANObjectCache object [optional]
-        *  - hostname : the name of the current server [optional]
+        *  - hostname : The name of the current server [optional]
+        *  - cliMode: Whether the execution context is a CLI script. [optional]
+        *  - profiler : Class name or instance with profileIn()/profileOut() methods. [optional]
+        *  - trxProfiler: TransactionProfiler instance. [optional]
+        *  - replLogger: PSR-3 logger instance. [optional]
+        *  - connLogger: PSR-3 logger instance. [optional]
+        *  - queryLogger: PSR-3 logger instance. [optional]
+        *  - perfLogger: PSR-3 logger instance. [optional]
+        *  - errorLogger : Callback that takes an Exception and logs it. [optional]
         * @throws InvalidArgumentException
         */
        public function __construct( array $params );