Merge "for exports, make sure we compare page titles as strings only"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Sat, 6 Apr 2019 18:53:54 +0000 (18:53 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Sat, 6 Apr 2019 18:53:54 +0000 (18:53 +0000)
24 files changed:
.phan/config.php
.phpcs.xml
autoload.php
includes/MovePage.php
includes/OutputPage.php
includes/gallery/PackedHoverImageGallery.php [new file with mode: 0644]
includes/gallery/PackedOverlayImageGallery.php
includes/htmlform/fields/HTMLSelectAndOtherField.php
includes/libs/IP.php
includes/libs/rdbms/database/DBConnRef.php
includes/libs/rdbms/database/Database.php
includes/libs/rdbms/database/IDatabase.php
includes/libs/rdbms/database/MaintainableDBConnRef.php
includes/libs/rdbms/exception/DBReadOnlyRoleError.php [new file with mode: 0644]
includes/libs/rdbms/loadbalancer/LoadBalancer.php
includes/resourceloader/ResourceLoaderFileModule.php
includes/resourceloader/ResourceLoaderImage.php
includes/resourceloader/ResourceLoaderOOUIFileModule.php
includes/resourceloader/ResourceLoaderOOUIModule.php
includes/resourceloader/ResourceLoaderSkinModule.php
includes/resourceloader/ResourceLoaderWikiModule.php
languages/messages/MessagesEn.php
tests/phpunit/includes/db/LoadBalancerTest.php
tests/phpunit/includes/libs/rdbms/database/DBConnRefTest.php

index 44ebedc..37b2153 100644 (file)
@@ -147,10 +147,6 @@ $cfg['suppress_issue_types'] = array_merge( $cfg['suppress_issue_types'], [
        "PhanUndeclaredVariableAssignOp",
        // approximate error count: 55
        "PhanUndeclaredVariableDim",
-       // approximate error count: 3
-       "PhanUnextractableAnnotationElementName",
-       // approximate error count: 4
-       "PhanUnextractableAnnotationSuffix",
 ] );
 
 $cfg['ignore_undeclared_variables_in_global_scope'] = true;
index 9939c54..170e16d 100644 (file)
                <exclude-pattern>*/includes/diff/DairikiDiff\.php</exclude-pattern>
                <exclude-pattern>*/includes/Feed\.php</exclude-pattern>
                <exclude-pattern>*/includes/filerepo/file/LocalFile\.php</exclude-pattern>
-               <exclude-pattern>*/includes/gallery/PackedOverlayImageGallery\.php</exclude-pattern>
                <exclude-pattern>*/includes/htmlform/HTMLFormElement\.php</exclude-pattern>
                <exclude-pattern>*/includes/libs/filebackend/FileBackendStore\.php</exclude-pattern>
                <exclude-pattern>*/includes/libs/filebackend/FSFileBackend\.php</exclude-pattern>
index bb1b3b2..b22aeab 100644 (file)
@@ -1070,7 +1070,7 @@ $wgAutoloadLocalClasses = [
        'PPNode_Hash_Tree' => __DIR__ . '/includes/parser/Preprocessor_Hash.php',
        'PPTemplateFrame_DOM' => __DIR__ . '/includes/parser/Preprocessor_DOM.php',
        'PPTemplateFrame_Hash' => __DIR__ . '/includes/parser/Preprocessor_Hash.php',
-       'PackedHoverImageGallery' => __DIR__ . '/includes/gallery/PackedOverlayImageGallery.php',
+       'PackedHoverImageGallery' => __DIR__ . '/includes/gallery/PackedHoverImageGallery.php',
        'PackedImageGallery' => __DIR__ . '/includes/gallery/PackedImageGallery.php',
        'PackedOverlayImageGallery' => __DIR__ . '/includes/gallery/PackedOverlayImageGallery.php',
        'Page' => __DIR__ . '/includes/page/Page.php',
@@ -1636,6 +1636,7 @@ $wgAutoloadLocalClasses = [
        'Wikimedia\\Rdbms\\DBQueryError' => __DIR__ . '/includes/libs/rdbms/exception/DBQueryError.php',
        'Wikimedia\\Rdbms\\DBQueryTimeoutError' => __DIR__ . '/includes/libs/rdbms/exception/DBQueryTimeoutError.php',
        'Wikimedia\\Rdbms\\DBReadOnlyError' => __DIR__ . '/includes/libs/rdbms/exception/DBReadOnlyError.php',
+       'Wikimedia\\Rdbms\\DBReadOnlyRoleError' => __DIR__ . '/includes/libs/rdbms/exception/DBReadOnlyRoleError.php',
        'Wikimedia\\Rdbms\\DBReplicationWaitError' => __DIR__ . '/includes/libs/rdbms/exception/DBReplicationWaitError.php',
        'Wikimedia\\Rdbms\\DBTransactionError' => __DIR__ . '/includes/libs/rdbms/exception/DBTransactionError.php',
        'Wikimedia\\Rdbms\\DBTransactionSizeError' => __DIR__ . '/includes/libs/rdbms/exception/DBTransactionSizeError.php',
index db5750a..2edd669 100644 (file)
@@ -427,8 +427,8 @@ class MovePage {
         * Can also be used to revert after a DB failure.
         *
         * @private
-        * @param Title Old location to move the file from.
-        * @param Title New location to move the file to.
+        * @param Title $oldTitle Old location to move the file from.
+        * @param Title $newTitle New location to move the file to.
         * @return Status
         */
        private function moveFile( $oldTitle, $newTitle ) {
index 786ecc4..cb3f1ad 100644 (file)
@@ -2187,7 +2187,7 @@ class OutputPage extends ContextSource {
         * Parse wikitext and return the HTML (internal implementation helper)
         *
         * @param string $text
-        * @param Title The title to use
+        * @param Title $title The title to use
         * @param bool $linestart Is this the start of a line?
         * @param bool $tidy Whether the output should be tidied
         * @param bool $interface Use interface language (instead of content language) while parsing
diff --git a/includes/gallery/PackedHoverImageGallery.php b/includes/gallery/PackedHoverImageGallery.php
new file mode 100644 (file)
index 0000000..2e1ef7d
--- /dev/null
@@ -0,0 +1,28 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+/**
+ * Same as Packed except different CSS is applied to make the
+ * caption only show up on hover. If a touch screen is detected,
+ * falls back to PackedHoverGallery. Degrades gracefully for
+ * screen readers.
+ */
+class PackedHoverImageGallery extends PackedOverlayImageGallery {
+}
index 0a5a457..4c72f87 100644 (file)
@@ -53,12 +53,3 @@ class PackedOverlayImageGallery extends PackedImageGallery {
                        . "\n\t\t\t</div></div>";
        }
 }
-
-/**
- * Same as Packed except different CSS is applied to make the
- * caption only show up on hover. If a touch screen is detected,
- * falls back to PackedHoverGallery. Degrades gracefully for
- * screen readers.
- */
-class PackedHoverImageGallery extends PackedOverlayImageGallery {
-}
index 4e64e9d..f137bf1 100644 (file)
@@ -144,7 +144,7 @@ class HTMLSelectAndOtherField extends HTMLSelectField {
        /**
         * @param WebRequest $request
         *
-        * @return array("<overall message>","<select value>","<text field value>")
+        * @return array ["<overall message>","<select value>","<text field value>"]
         */
        public function loadDataFromRequest( $request ) {
                if ( $request->getCheck( $this->mName ) ) {
index 8efcd15..da525e7 100644 (file)
@@ -467,7 +467,7 @@ class IP {
         * to an integer network and a number of bits
         *
         * @param string $range IP with CIDR prefix
-        * @return array(int or string, int)
+        * @return array [int or string, int]
         */
        public static function parseCIDR( $range ) {
                if ( self::isIPv6( $range ) ) {
@@ -557,7 +557,7 @@ class IP {
         *
         * @param string $range
         *
-        * @return array(string, int)
+        * @return array [string, int]
         */
        private static function parseCIDR6( $range ) {
                # Explode into <expanded IP,range>
@@ -598,7 +598,7 @@ class IP {
         *
         * @param string $range
         *
-        * @return array(string, string)
+        * @return array [string, string]
         */
        private static function parseRange6( $range ) {
                # Expand any IPv6 IP
index cf582b7..f3ab1c5 100644 (file)
@@ -8,7 +8,6 @@ use InvalidArgumentException;
  * Helper class to handle automatically marking connections as reusable (via RAII pattern)
  * as well handling deferring the actual network connection until the handle is used
  *
- * @note: proxy methods are defined explicitly to avoid interface errors
  * @ingroup Database
  * @since 1.22
  */
@@ -19,6 +18,8 @@ class DBConnRef implements IDatabase {
        private $conn;
        /** @var array|null N-tuple of (server index, group, DatabaseDomain|string) */
        private $params;
+       /** @var int One of DB_MASTER/DB_REPLICA */
+       private $role;
 
        const FLD_INDEX = 0;
        const FLD_GROUP = 1;
@@ -27,10 +28,13 @@ class DBConnRef implements IDatabase {
 
        /**
         * @param ILoadBalancer $lb Connection manager for $conn
-        * @param Database|array $conn Database handle or (server index, query groups, domain, flags)
+        * @param Database|array $conn Database or (server index, query groups, domain, flags)
+        * @param int $role The type of connection asked for; one of DB_MASTER/DB_REPLICA
+        * @internal This method should not be called outside of LoadBalancer
         */
-       public function __construct( ILoadBalancer $lb, $conn ) {
+       public function __construct( ILoadBalancer $lb, $conn, $role ) {
                $this->lb = $lb;
+               $this->role = $role;
                if ( $conn instanceof Database ) {
                        $this->conn = $conn; // live handle
                } elseif ( is_array( $conn ) && count( $conn ) >= 4 && $conn[self::FLD_DOMAIN] !== false ) {
@@ -49,6 +53,14 @@ class DBConnRef implements IDatabase {
                return $this->conn->$name( ...$arguments );
        }
 
+       /**
+        * @return int DB_MASTER when this *requires* the master DB, otherwise DB_REPLICA
+        * @since 1.33
+        */
+       public function getReferenceRole() {
+               return $this->role;
+       }
+
        public function getServerInfo() {
                return $this->__call( __FUNCTION__, func_get_args() );
        }
@@ -255,7 +267,11 @@ class DBConnRef implements IDatabase {
        }
 
        public function query( $sql, $fname = __METHOD__, $flags = 0 ) {
-               return $this->__call( __FUNCTION__, func_get_args() );
+               if ( $this->role !== ILoadBalancer::DB_MASTER ) {
+                       $flags |= IDatabase::QUERY_REPLICA_ROLE;
+               }
+
+               return $this->__call( __FUNCTION__, [ $sql, $fname, $flags ] );
        }
 
        public function freeResult( $res ) {
@@ -310,6 +326,8 @@ class DBConnRef implements IDatabase {
        public function lockForUpdate(
                $table, $conds = '', $fname = __METHOD__, $options = [], $join_conds = []
        ) {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
@@ -326,10 +344,14 @@ class DBConnRef implements IDatabase {
        }
 
        public function insert( $table, $a, $fname = __METHOD__, $options = [] ) {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
        public function update( $table, $values, $conds, $fname = __METHOD__, $options = [] ) {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
@@ -435,26 +457,36 @@ class DBConnRef implements IDatabase {
        }
 
        public function nextSequenceValue( $seqName ) {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
        public function replace( $table, $uniqueIndexes, $rows, $fname = __METHOD__ ) {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
        public function upsert(
                $table, array $rows, $uniqueIndexes, array $set, $fname = __METHOD__
        ) {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
        public function deleteJoin(
                $delTable, $joinTable, $delVar, $joinVar, $conds, $fname = __METHOD__
        ) {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
        public function delete( $table, $conds, $fname = __METHOD__ ) {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
@@ -462,6 +494,8 @@ class DBConnRef implements IDatabase {
                $destTable, $srcTable, $varMap, $conds,
                $fname = __METHOD__, $insertOptions = [], $selectOptions = [], $selectJoinConds = []
        ) {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
@@ -529,18 +563,21 @@ class DBConnRef implements IDatabase {
        }
 
        public function onTransactionResolution( callable $callback, $fname = __METHOD__ ) {
+               // DB_REPLICA role: caller might want to refresh cache after a REPEATABLE-READ snapshot
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
        public function onTransactionCommitOrIdle( callable $callback, $fname = __METHOD__ ) {
+               // DB_REPLICA role: caller might want to refresh cache after a REPEATABLE-READ snapshot
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
        public function onTransactionIdle( callable $callback, $fname = __METHOD__ ) {
-               return $this->__call( __FUNCTION__, func_get_args() );
+               return $this->onTransactionCommitOrIdle( $callback, $fname );
        }
 
        public function onTransactionPreCommitOrIdle( callable $callback, $fname = __METHOD__ ) {
+               // DB_REPLICA role: caller might want to refresh cache after a cache mutex is released
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
@@ -551,20 +588,24 @@ class DBConnRef implements IDatabase {
        public function startAtomic(
                $fname = __METHOD__, $cancelable = IDatabase::ATOMIC_NOT_CANCELABLE
        ) {
+               // Don't call assertRoleAllowsWrites(); caller might want a REPEATABLE-READ snapshot
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
        public function endAtomic( $fname = __METHOD__ ) {
+               // Don't call assertRoleAllowsWrites(); caller might want a REPEATABLE-READ snapshot
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
        public function cancelAtomic( $fname = __METHOD__, AtomicSectionIdentifier $sectionId = null ) {
+               // Don't call assertRoleAllowsWrites(); caller might want a REPEATABLE-READ snapshot
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
        public function doAtomicSection(
                $fname, callable $callback, $cancelable = self::ATOMIC_NOT_CANCELABLE
        ) {
+               // Don't call assertRoleAllowsWrites(); caller might want a REPEATABLE-READ snapshot
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
@@ -627,18 +668,26 @@ class DBConnRef implements IDatabase {
        }
 
        public function lockIsFree( $lockName, $method ) {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
        public function lock( $lockName, $method, $timeout = 5 ) {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
        public function unlock( $lockName, $method ) {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
        public function getScopedLockAndFlush( $lockKey, $fname, $timeout ) {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
@@ -674,6 +723,26 @@ class DBConnRef implements IDatabase {
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
+       /**
+        * Error out if the role is not DB_MASTER
+        *
+        * Note that the underlying connection may or may not itself be read-only.
+        * It could even be to a writable master (both server-side and to the application).
+        * This error is meant for the case when a DB_REPLICA handle was requested but a
+        * a write was attempted on that handle regardless.
+        *
+        * In configurations where the master DB has some generic read load or is the only server,
+        * DB_MASTER/DB_REPLICA will sometimes (or always) use the same connection to the master DB.
+        * This does not effect the role of DBConnRef instances.
+        * @throws DBReadOnlyRoleError
+        */
+       protected function assertRoleAllowsWrites() {
+               // DB_MASTER is "prima facie" writable
+               if ( $this->role !== ILoadBalancer::DB_MASTER ) {
+                       throw new DBReadOnlyRoleError( $this->conn, "Cannot write with role DB_REPLICA" );
+               }
+       }
+
        /**
         * Clean up the connection when out of scope
         */
index a839946..c5ef758 100644 (file)
@@ -1032,7 +1032,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         */
        protected function assertIsWritableMaster() {
                if ( $this->getLBInfo( 'replica' ) === true ) {
-                       throw new DBUnexpectedError(
+                       throw new DBReadOnlyRoleError(
                                $this,
                                'Write operations are not allowed on replica database connections.'
                        );
@@ -1194,7 +1194,6 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 
                $flags = (int)$flags; // b/c; this field used to be a bool
                $ignoreErrors = $this->hasFlags( $flags, self::QUERY_SILENCE_ERRORS );
-               $pseudoPermanent = $this->hasFlags( $flags, self::QUERY_PSEUDO_PERMANENT );
 
                $priorTransaction = $this->trxLevel;
                $priorWritesPending = $this->writesOrCallbacksPending();
@@ -1206,8 +1205,13 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        $this->assertIsWritableMaster();
                        # Do not treat temporary table writes as "meaningful writes" that need committing.
                        # Profile them as reads. Integration tests can override this behavior via $flags.
+                       $pseudoPermanent = $this->hasFlags( $flags, self::QUERY_PSEUDO_PERMANENT );
                        $tableType = $this->registerTempTableWrite( $sql, $pseudoPermanent );
                        $isEffectiveWrite = ( $tableType !== self::TEMP_NORMAL );
+                       # DBConnRef uses QUERY_REPLICA_ROLE to enforce the replica role for raw SQL queries
+                       if ( $isEffectiveWrite && $this->hasFlags( $flags, self::QUERY_REPLICA_ROLE ) ) {
+                               throw new DBReadOnlyRoleError( $this, "Cannot write; target role is DB_REPLICA" );
+                       }
                } else {
                        $isEffectiveWrite = false;
                }
index 90b888d..a8d4f1e 100644 (file)
@@ -113,6 +113,8 @@ interface IDatabase {
         *   permanent as far as write tracking is concerned. This is useful for testing.
         */
        const QUERY_PSEUDO_PERMANENT = 2;
+       /** @var int Enforce that a query does not make effective writes */
+       const QUERY_REPLICA_ROLE = 4;
 
        /** @var bool Parameter to unionQueries() for UNION ALL */
        const UNION_ALL = true;
index 8a2c795..10a0897 100644 (file)
@@ -30,6 +30,8 @@ class MaintainableDBConnRef extends DBConnRef implements IMaintainableDatabase {
                $fname = false,
                callable $inputCallback = null
        ) {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
@@ -40,14 +42,20 @@ class MaintainableDBConnRef extends DBConnRef implements IMaintainableDatabase {
                $fname = __METHOD__,
                callable $inputCallback = null
        ) {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
        public function dropTable( $tableName, $fName = __METHOD__ ) {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
        public function deadlockLoop() {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
@@ -66,6 +74,8 @@ class MaintainableDBConnRef extends DBConnRef implements IMaintainableDatabase {
        public function duplicateTableStructure(
                $oldName, $newName, $temporary = false, $fname = __METHOD__
        ) {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
@@ -74,10 +84,14 @@ class MaintainableDBConnRef extends DBConnRef implements IMaintainableDatabase {
        }
 
        public function lockTables( array $read, array $write, $method ) {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
        public function unlockTables( $method ) {
+               $this->assertRoleAllowsWrites();
+
                return $this->__call( __FUNCTION__, func_get_args() );
        }
 
diff --git a/includes/libs/rdbms/exception/DBReadOnlyRoleError.php b/includes/libs/rdbms/exception/DBReadOnlyRoleError.php
new file mode 100644 (file)
index 0000000..4d565ba
--- /dev/null
@@ -0,0 +1,30 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ * @ingroup Database
+ */
+
+namespace Wikimedia\Rdbms;
+
+/**
+ * Exception class for attempted DB write access to a DBConnRef with the DB_REPLICA role
+ *
+ * @ingroup Database
+ */
+class DBReadOnlyRoleError extends DBUnexpectedError {
+}
index bd22aca..da5382a 100644 (file)
@@ -834,23 +834,36 @@ class LoadBalancer implements ILoadBalancer {
                }
        }
 
-       public function getConnectionRef( $db, $groups = [], $domain = false, $flags = 0 ) {
+       public function getConnectionRef( $i, $groups = [], $domain = false, $flags = 0 ) {
                $domain = $this->resolveDomainID( $domain );
+               $role = $this->getRoleFromIndex( $i );
 
-               return new DBConnRef( $this, $this->getConnection( $db, $groups, $domain, $flags ) );
+               return new DBConnRef( $this, $this->getConnection( $i, $groups, $domain, $flags ), $role );
        }
 
-       public function getLazyConnectionRef( $db, $groups = [], $domain = false, $flags = 0 ) {
+       public function getLazyConnectionRef( $i, $groups = [], $domain = false, $flags = 0 ) {
                $domain = $this->resolveDomainID( $domain );
+               $role = $this->getRoleFromIndex( $i );
 
-               return new DBConnRef( $this, [ $db, $groups, $domain, $flags ] );
+               return new DBConnRef( $this, [ $i, $groups, $domain, $flags ], $role );
        }
 
-       public function getMaintenanceConnectionRef( $db, $groups = [], $domain = false, $flags = 0 ) {
+       public function getMaintenanceConnectionRef( $i, $groups = [], $domain = false, $flags = 0 ) {
                $domain = $this->resolveDomainID( $domain );
+               $role = $this->getRoleFromIndex( $i );
 
                return new MaintainableDBConnRef(
-                       $this, $this->getConnection( $db, $groups, $domain, $flags ) );
+                       $this, $this->getConnection( $i, $groups, $domain, $flags ), $role );
+       }
+
+       /**
+        * @param int $i Server index or DB_MASTER/DB_REPLICA
+        * @return int One of DB_MASTER/DB_REPLICA
+        */
+       private function getRoleFromIndex( $i ) {
+               return ( $i === self::DB_MASTER || $i === $this->getWriterIndex() )
+                       ? self::DB_MASTER
+                       : self::DB_REPLICA;
        }
 
        public function openConnection( $i, $domain = false, $flags = 0 ) {
index 9be5de3..031541b 100644 (file)
@@ -385,7 +385,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                if ( $this->packageFiles !== null ) {
                        $packageFiles = $this->getPackageFiles( $context );
                        if ( $deprecationScript ) {
-                               $mainFile =& $packageFiles['files'][ $packageFiles['main'] ];
+                               $mainFile =& $packageFiles['files'][$packageFiles['main']];
                                $mainFile['content'] = $deprecationScript . $mainFile['content'];
                        }
                        return $packageFiles;
index 8cd5b19..d10be12 100644 (file)
@@ -73,7 +73,7 @@ class ResourceLoaderImage {
                }
                // Remove 'deprecated' key
                if ( is_array( $this->descriptor ) ) {
-                       unset( $this->descriptor[ 'deprecated' ] );
+                       unset( $this->descriptor['deprecated'] );
                }
 
                // Ensure that all files have common extension.
index e97e074..7d39a58 100644 (file)
@@ -28,15 +28,15 @@ class ResourceLoaderOOUIFileModule extends ResourceLoaderFileModule {
        use ResourceLoaderOOUIModule;
 
        public function __construct( $options = [] ) {
-               if ( isset( $options[ 'themeScripts' ] ) ) {
-                       $skinScripts = $this->getSkinSpecific( $options[ 'themeScripts' ], 'scripts' );
+               if ( isset( $options['themeScripts'] ) ) {
+                       $skinScripts = $this->getSkinSpecific( $options['themeScripts'], 'scripts' );
                        if ( !isset( $options['skinScripts'] ) ) {
                                $options['skinScripts'] = [];
                        }
                        $this->extendSkinSpecific( $options['skinScripts'], $skinScripts );
                }
-               if ( isset( $options[ 'themeStyles' ] ) ) {
-                       $skinStyles = $this->getSkinSpecific( $options[ 'themeStyles' ], 'styles' );
+               if ( isset( $options['themeStyles'] ) ) {
+                       $skinStyles = $this->getSkinSpecific( $options['themeStyles'], 'styles' );
                        if ( !isset( $options['skinStyles'] ) ) {
                                $options['skinStyles'] = [];
                        }
index 0a4e94e..0395127 100644 (file)
@@ -104,7 +104,7 @@ trait ResourceLoaderOOUIModule {
         */
        protected function getThemePath( $theme, $kind, $module ) {
                $paths = self::getThemePaths();
-               $path = $paths[ $theme ][ $kind ];
+               $path = $paths[$theme][$kind];
                $path = str_replace( '{module}', $module, $path );
                return $path;
        }
index 6393803..2dd6c17 100644 (file)
@@ -98,14 +98,14 @@ class ResourceLoaderSkinModule extends ResourceLoaderFileModule {
 
                if ( !is_array( $logo ) ) {
                        // No media queries required if we only have one variant
-                       $preloadLinks[ $logo ] = [ 'as' => 'image' ];
+                       $preloadLinks[$logo] = [ 'as' => 'image' ];
                        return $preloadLinks;
                }
 
                if ( isset( $logo['svg'] ) ) {
                        // No media queries required if we only have a 1x and svg variant
                        // because all preload-capable browsers support SVGs
-                       $preloadLinks [ $logo['svg'] ] = [ 'as' => 'image' ];
+                       $preloadLinks[$logo['svg']] = [ 'as' => 'image' ];
                        return $preloadLinks;
                }
 
@@ -124,7 +124,10 @@ class ResourceLoaderSkinModule extends ResourceLoaderFileModule {
                } );
 
                foreach ( $logosPerDppx as $dppx => $src ) {
-                       $logos[] = [ 'dppx' => $dppx, 'src' => $src ];
+                       $logos[] = [
+                               'dppx' => $dppx,
+                               'src' => $src
+                       ];
                }
 
                $logosCount = count( $logos );
@@ -138,21 +141,24 @@ class ResourceLoaderSkinModule extends ResourceLoaderFileModule {
                                // Smallest dppx
                                // min-resolution is ">=" (larger than or equal to)
                                // "not min-resolution" is essentially "<"
-                               $media_query = 'not all and (min-resolution: ' . $logos[ 1 ]['dppx'] . 'dppx)';
+                               $media_query = 'not all and (min-resolution: ' . $logos[1]['dppx'] . 'dppx)';
                        } elseif ( $i !== $logosCount - 1 ) {
                                // In between
                                // Media query expressions can only apply "not" to the entire expression
                                // (e.g. can't express ">= 1.5 and not >= 2).
                                // Workaround: Use <= 1.9999 in place of < 2.
-                               $upper_bound = floatval( $logos[ $i + 1 ]['dppx'] ) - 0.000001;
-                               $media_query = '(min-resolution: ' . $logos[ $i ]['dppx'] .
+                               $upper_bound = floatval( $logos[$i + 1]['dppx'] ) - 0.000001;
+                               $media_query = '(min-resolution: ' . $logos[$i]['dppx'] .
                                        'dppx) and (max-resolution: ' . $upper_bound . 'dppx)';
                        } else {
                                // Largest dppx
-                               $media_query = '(min-resolution: ' . $logos[ $i ]['dppx'] . 'dppx)';
+                               $media_query = '(min-resolution: ' . $logos[$i]['dppx'] . 'dppx)';
                        }
 
-                       $preloadLinks[ $logos[$i]['src'] ] = [ 'as' => 'image', 'media' => $media_query ];
+                       $preloadLinks[$logos[$i]['src']] = [
+                               'as' => 'image',
+                               'media' => $media_query
+                       ];
                }
 
                return $preloadLinks;
index a91537f..9fad348 100644 (file)
@@ -433,7 +433,7 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule {
                                // Avoid including ids or timestamps of revision/page tables so
                                // that versions are not wasted
                                $title = new TitleValue( (int)$row->page_namespace, $row->page_title );
-                               $titleInfo[ self::makeTitleKey( $title ) ] = [
+                               $titleInfo[self::makeTitleKey( $title )] = [
                                        'page_len' => $row->page_len,
                                        'page_latest' => $row->page_latest,
                                        'page_touched' => $row->page_touched,
index f325641..86ac01a 100644 (file)
@@ -438,7 +438,7 @@ $specialPageAliases = [
        'Listgrants'                => [ 'ListGrants' ],
        'Listredirects'             => [ 'ListRedirects' ],
        'ListDuplicatedFiles'       => [ 'ListDuplicatedFiles', 'ListFileDuplicates' ],
-       'Listusers'                 => [ 'ListUsers', 'UserList' ],
+       'Listusers'                 => [ 'ListUsers', 'UserList', 'Users' ],
        'Lockdb'                    => [ 'LockDB' ],
        'Log'                       => [ 'Log', 'Logs' ],
        'Lonelypages'               => [ 'LonelyPages', 'OrphanedPages' ],
index cc01c7d..4291bcc 100644 (file)
@@ -307,11 +307,16 @@ class LoadBalancerTest extends MediaWikiTestCase {
 
                $i = $lb->getWriterIndex();
                $this->assertEquals( null, $lb->getAnyOpenConnection( $i ) );
+
                $conn1 = $lb->getConnection( $i );
                $this->assertNotEquals( null, $conn1 );
                $this->assertEquals( $conn1, $lb->getAnyOpenConnection( $i ) );
+               $this->assertFalse( $conn1->getFlag( DBO_TRX ) );
+
                $conn2 = $lb->getConnection( $i, [], false, $lb::CONN_TRX_AUTOCOMMIT );
                $this->assertNotEquals( null, $conn2 );
+               $this->assertFalse( $conn2->getFlag( DBO_TRX ) );
+
                if ( $lb->getServerAttributes( $i )[Database::ATTR_DB_LEVEL_LOCKING] ) {
                        $this->assertEquals( null,
                                $lb->getAnyOpenConnection( $i, $lb::CONN_TRX_AUTOCOMMIT ) );
@@ -355,7 +360,7 @@ class LoadBalancerTest extends MediaWikiTestCase {
                                'type' => $wgDBtype,
                                'dbDirectory' => $wgSQLiteDataDir,
                                'load' => 0,
-                               'flags' => DBO_TRX // REPEATABLE-READ for consistency
+                               'flags' => DBO_TRX // simulate a web request with DBO_TRX
                        ],
                ];
 
@@ -428,4 +433,60 @@ class LoadBalancerTest extends MediaWikiTestCase {
                $conn1->close();
                $conn2->close();
        }
+
+       public function testDBConnRefReadsMasterAndReplicaRoles() {
+               $lb = $this->newSingleServerLocalLoadBalancer();
+
+               $rConn = $lb->getConnectionRef( DB_REPLICA );
+               $wConn = $lb->getConnectionRef( DB_MASTER );
+               $wConn2 = $lb->getConnectionRef( 0 );
+
+               $v = [ 'value' => '1', '1' ];
+               $sql = 'SELECT MAX(1) AS value';
+               foreach ( [ $rConn, $wConn, $wConn2 ] as $conn ) {
+                       $conn->clearFlag( $conn::DBO_TRX );
+
+                       $res = $conn->query( $sql, __METHOD__ );
+                       $this->assertEquals( $v, $conn->fetchRow( $res ) );
+
+                       $res = $conn->query( $sql, __METHOD__, $conn::QUERY_REPLICA_ROLE );
+                       $this->assertEquals( $v, $conn->fetchRow( $res ) );
+               }
+
+               $wConn->getScopedLockAndFlush( 'key', __METHOD__, 1 );
+               $wConn2->getScopedLockAndFlush( 'key2', __METHOD__, 1 );
+       }
+
+       /**
+        * @expectedException \Wikimedia\Rdbms\DBReadOnlyRoleError
+        */
+       public function testDBConnRefWritesReplicaRole() {
+               $lb = $this->newSingleServerLocalLoadBalancer();
+
+               $rConn = $lb->getConnectionRef( DB_REPLICA );
+
+               $rConn->query( 'DELETE FROM sometesttable WHERE 1=0' );
+       }
+
+       /**
+        * @expectedException \Wikimedia\Rdbms\DBReadOnlyRoleError
+        */
+       public function testDBConnRefWritesReplicaRoleIndex() {
+               $lb = $this->newMultiServerLocalLoadBalancer();
+
+               $rConn = $lb->getConnectionRef( 1 );
+
+               $rConn->query( 'DELETE FROM sometesttable WHERE 1=0' );
+       }
+
+       /**
+        * @expectedException \Wikimedia\Rdbms\DBReadOnlyRoleError
+        */
+       public function testDBConnRefWritesReplicaRoleInsert() {
+               $lb = $this->newMultiServerLocalLoadBalancer();
+
+               $rConn = $lb->getConnectionRef( DB_REPLICA );
+
+               $rConn->insert( 'test', [ 't' => 1 ], __METHOD__ );
+       }
 }
index 9b72b95..33e5c3b 100644 (file)
@@ -75,12 +75,12 @@ class DBConnRefTest extends PHPUnit\Framework\TestCase {
         */
        private function getDBConnRef( ILoadBalancer $lb = null ) {
                $lb = $lb ?: $this->getLoadBalancerMock();
-               return new DBConnRef( $lb, $this->getDatabaseMock() );
+               return new DBConnRef( $lb, $this->getDatabaseMock(), DB_MASTER );
        }
 
        public function testConstruct() {
                $lb = $this->getLoadBalancerMock();
-               $ref = new DBConnRef( $lb, $this->getDatabaseMock() );
+               $ref = new DBConnRef( $lb, $this->getDatabaseMock(), DB_MASTER );
 
                $this->assertInstanceOf( ResultWrapper::class, $ref->select( 'whatever', '*' ) );
        }
@@ -99,10 +99,19 @@ class DBConnRefTest extends PHPUnit\Framework\TestCase {
 
                $ref = new DBConnRef(
                        $lb,
-                       [ DB_MASTER, [ 'test' ], 'dummy', ILoadBalancer::CONN_TRX_AUTOCOMMIT ]
+                       [ DB_MASTER, [ 'test' ], 'dummy', ILoadBalancer::CONN_TRX_AUTOCOMMIT ],
+                       DB_MASTER
                );
 
                $this->assertInstanceOf( ResultWrapper::class, $ref->select( 'whatever', '*' ) );
+               $this->assertEquals( DB_MASTER, $ref->getReferenceRole() );
+
+               $ref2 = new DBConnRef(
+                       $lb,
+                       [ DB_MASTER, [ 'test' ], 'dummy', ILoadBalancer::CONN_TRX_AUTOCOMMIT ],
+                       DB_REPLICA
+               );
+               $this->assertEquals( DB_REPLICA, $ref2->getReferenceRole() );
        }
 
        public function testDestruct() {
@@ -124,7 +133,7 @@ class DBConnRefTest extends PHPUnit\Framework\TestCase {
                $this->setExpectedException( InvalidArgumentException::class, '' );
 
                $lb = $this->getLoadBalancerMock();
-               new DBConnRef( $lb, 17 ); // bad constructor argument
+               new DBConnRef( $lb, 17, DB_REPLICA ); // bad constructor argument
        }
 
        /**
@@ -137,7 +146,7 @@ class DBConnRefTest extends PHPUnit\Framework\TestCase {
                $lb->expects( $this->never() )
                        ->method( 'getConnection' );
 
-               $ref = new DBConnRef( $lb, [ DB_REPLICA, [], 'dummy', 0 ] );
+               $ref = new DBConnRef( $lb, [ DB_REPLICA, [], 'dummy', 0 ], DB_REPLICA );
 
                $this->assertSame( 'dummy', $ref->getDomainID() );
        }
@@ -156,7 +165,7 @@ class DBConnRefTest extends PHPUnit\Framework\TestCase {
                $this->assertInternalType( 'string', $ref->__toString() );
 
                $lb = $this->getLoadBalancerMock();
-               $ref = new DBConnRef( $lb, [ DB_MASTER, [], 'test', 0 ] );
+               $ref = new DBConnRef( $lb, [ DB_MASTER, [], 'test', 0 ], DB_MASTER );
                $this->assertInternalType( 'string', $ref->__toString() );
        }
 
@@ -166,7 +175,49 @@ class DBConnRefTest extends PHPUnit\Framework\TestCase {
         */
        public function testClose() {
                $lb = $this->getLoadBalancerMock();
-               $ref = new DBConnRef( $lb, [ DB_REPLICA, [], 'dummy', 0 ] );
+               $ref = new DBConnRef( $lb, [ DB_REPLICA, [], 'dummy', 0 ], DB_MASTER );
                $ref->close();
        }
+
+       /**
+        * @covers Wikimedia\Rdbms\DBConnRef::getReferenceRole
+        */
+       public function testGetReferenceRole() {
+               $lb = $this->getLoadBalancerMock();
+               $ref = new DBConnRef( $lb, [ DB_REPLICA, [], 'dummy', 0 ], DB_REPLICA );
+               $this->assertSame( DB_REPLICA, $ref->getReferenceRole() );
+
+               $ref = new DBConnRef( $lb, [ DB_MASTER, [], 'dummy', 0 ], DB_MASTER );
+               $this->assertSame( DB_MASTER, $ref->getReferenceRole() );
+
+               $ref = new DBConnRef( $lb, [ 1, [], 'dummy', 0 ], DB_REPLICA );
+               $this->assertSame( DB_REPLICA, $ref->getReferenceRole() );
+
+               $ref = new DBConnRef( $lb, [ 0, [], 'dummy', 0 ], DB_MASTER );
+               $this->assertSame( DB_MASTER, $ref->getReferenceRole() );
+       }
+
+       /**
+        * @covers Wikimedia\Rdbms\DBConnRef::getReferenceRole
+        * @expectedException Wikimedia\Rdbms\DBReadOnlyRoleError
+        * @dataProvider provideRoleExceptions
+        */
+       public function testRoleExceptions( $method, $args ) {
+               $lb = $this->getLoadBalancerMock();
+               $ref = new DBConnRef( $lb, [ DB_REPLICA, [], 'dummy', 0 ], DB_REPLICA );
+               $ref->$method( ...$args );
+       }
+
+       function provideRoleExceptions() {
+               return [
+                       [ 'insert', [ 'table', [ 'a' => 1 ] ] ],
+                       [ 'update', [ 'table', [ 'a' => 1 ], [ 'a' => 2 ] ] ],
+                       [ 'delete', [ 'table', [ 'a' => 1 ] ] ],
+                       [ 'replace', [ 'table', [ 'a' ], [ 'a' => 1 ] ] ],
+                       [ 'upsert', [ 'table', [ 'a' => 1 ], [ 'a' ], [ 'a = a + 1' ] ] ],
+                       [ 'lock', [ 'k', 'method' ] ],
+                       [ 'unlock', [ 'k', 'method' ] ],
+                       [ 'getScopedLockAndFlush', [ 'k', 'method', 1 ] ]
+               ];
+       }
 }