Add isCurrentWikiId()/isCurrentWikiDomain()/getCurrentWikiDomain() to WikiMap
authorAaron Schulz <aschulz@wikimedia.org>
Tue, 16 Oct 2018 02:18:16 +0000 (19:18 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Mon, 29 Oct 2018 21:53:37 +0000 (14:53 -0700)
Use these in place of various wfWikiID() calls.

Also cleanup UserRightsProxy wiki ID variable names and removed unused
and poorly named getDBname() method.

Change-Id: Ib28889663989382d845511f8d34712b08317f60e

17 files changed:
includes/MediaWiki.php
includes/SiteConfiguration.php
includes/WikiMap.php
includes/externalstore/ExternalStoreMwstore.php
includes/filebackend/FileBackendGroup.php
includes/filebackend/lockmanager/LockManagerGroup.php
includes/jobqueue/JobQueue.php
includes/jobqueue/JobQueueGroup.php
includes/jobqueue/jobs/RecentChangesUpdateJob.php
includes/mail/UserMailer.php
includes/page/WikiPage.php
includes/skins/Skin.php
includes/specials/SpecialUserrights.php
includes/user/LocalIdLookup.php
includes/user/User.php
includes/user/UserRightsProxy.php
tests/phpunit/includes/WikiMapTest.php

index bc57637..99b1a78 100644 (file)
@@ -684,9 +684,10 @@ class MediaWiki {
         */
        private static function getUrlDomainDistance( $url ) {
                $clusterWiki = WikiMap::getWikiFromUrl( $url );
-               if ( $clusterWiki === wfWikiID() ) {
+               if ( WikiMap::isCurrentWikiId( $clusterWiki ) ) {
                        return 'local'; // the current wiki
-               } elseif ( $clusterWiki !== false ) {
+               }
+               if ( $clusterWiki !== false ) {
                        return 'remote'; // another wiki in this cluster/farm
                }
 
index 1eaedc2..af65e45 100644 (file)
@@ -527,7 +527,7 @@ class SiteConfiguration {
 
                $multi = is_array( $settings );
                $settings = (array)$settings;
-               if ( $wiki === wfWikiID() ) { // $wiki is this wiki
+               if ( WikiMap::isCurrentWikiId( $wiki ) ) { // $wiki is this wiki
                        $res = [];
                        foreach ( $settings as $name ) {
                                if ( !preg_match( '/^wg[A-Z]/', $name ) ) {
index 9dc3bfe..b731d7b 100644 (file)
@@ -196,7 +196,8 @@ class WikiMap {
                                $infoMap = [];
                                // Make sure at least the current wiki is set, for simple configurations.
                                // This also makes it the first in the map, which is useful for common cases.
-                               $infoMap[wfWikiID()] = [
+                               $wikiId = self::getWikiIdFromDomain( self::getCurrentWikiDomain() );
+                               $infoMap[$wikiId] = [
                                        'url' => $wgCanonicalServer,
                                        'parts' => wfParseUrl( $wgCanonicalServer )
                                ];
@@ -250,12 +251,45 @@ class WikiMap {
         * @return string
         */
        public static function getWikiIdFromDomain( $domain ) {
-               if ( !( $domain instanceof DatabaseDomain ) ) {
-                       $domain = DatabaseDomain::newFromId( $domain );
-               }
+               $domain = DatabaseDomain::newFromId( $domain );
 
                return strlen( $domain->getTablePrefix() )
                        ? "{$domain->getDatabase()}-{$domain->getTablePrefix()}"
                        : $domain->getDatabase();
        }
+
+       /**
+        * @return DatabaseDomain Database domain of the current wiki
+        * @since 1.33
+        */
+       public static function getCurrentWikiDomain() {
+               global $wgDBname, $wgDBmwschema, $wgDBprefix;
+               // Avoid invoking LBFactory to avoid any chance of recursion
+               return new DatabaseDomain( $wgDBname, $wgDBmwschema, (string)$wgDBprefix );
+       }
+
+       /**
+        * @param DatabaseDomain|string $domain
+        * @return bool Whether $domain has the same DB/prefix as the current wiki
+        * @since 1.33
+        */
+       public static function isCurrentWikiDomain( $domain ) {
+               $domain = DatabaseDomain::newFromId( $domain );
+               $curDomain = self::getCurrentWikiDomain();
+
+               return (
+                       $curDomain->getDatabase() === $domain->getDatabase() &&
+                       // @TODO: check schema instead of assuming it's ""/"mediawiki" and never collides
+                       $curDomain->getTablePrefix() === $domain->getTablePrefix()
+               );
+       }
+
+       /**
+        * @param string $wikiId
+        * @return bool Whether $wikiId has the same DB/prefix as the current wiki
+        * @since 1.33
+        */
+       public static function isCurrentWikiId( $wikiId ) {
+               return ( self::getWikiIdFromDomain( self::getCurrentWikiDomain() ) === $wikiId );
+       }
 }
index c5893be..1bc2edc 100644 (file)
@@ -81,6 +81,7 @@ class ExternalStoreMwstore extends ExternalStoreMedium {
                        // Make sure ID is roughly lexicographically increasing for performance
                        $id = str_pad( UIDGenerator::newTimestampedUID128( 32 ), 26, '0', STR_PAD_LEFT );
                        // Segregate items by wiki ID for the sake of bookkeeping
+                       // @FIXME: this does not include the domain for b/c but it ideally should
                        $wiki = $this->params['wiki'] ?? wfWikiID();
 
                        $url = $be->getContainerStoragePath( 'data' ) . '/' . rawurlencode( $wiki );
index 503acdc..cbf9bff 100644 (file)
@@ -179,6 +179,7 @@ class FileBackendGroup {
                $config = $this->backends[$name]['config'];
                $config['class'] = $class;
                $config += [ // set defaults
+                       // @FIXME: this does not include the domain for b/c but it ideally should
                        'wikiId' => wfWikiID(), // e.g. "my_wiki-en_"
                        'mimeCallback' => [ $this, 'guessMimeInternal' ],
                        'obResetFunc' => 'wfResetOutputBuffers',
index 5d79dac..b8fb7fd 100644 (file)
@@ -50,7 +50,10 @@ class LockManagerGroup {
         * @return LockManagerGroup
         */
        public static function singleton( $domain = false ) {
-               $domain = ( $domain === false ) ? wfWikiID() : $domain;
+               if ( $domain === false ) {
+                       $domain = WikiMap::getCurrentWikiDomain()->getId();
+               }
+
                if ( !isset( self::$instances[$domain] ) ) {
                        self::$instances[$domain] = new self( $domain );
                        self::$instances[$domain]->initFromGlobals();
index c4ce2ba..22d5c33 100644 (file)
@@ -358,7 +358,7 @@ abstract class JobQueue {
                global $wgJobClasses;
 
                $this->assertNotReadOnly();
-               if ( $this->wiki !== wfWikiID() ) {
+               if ( !WikiMap::isCurrentWikiId( $this->wiki ) ) {
                        throw new MWException( "Cannot pop '{$this->type}' job off foreign wiki queue." );
                } elseif ( !isset( $wgJobClasses[$this->type] ) ) {
                        // Do not pop jobs if there is no class for the queue type
index 820c492..dc0b249 100644 (file)
@@ -78,7 +78,7 @@ class JobQueueGroup {
                        self::$instances[$wiki] = new self( $wiki, wfConfiguredReadOnlyReason() );
                        // Make sure jobs are not getting pushed to bogus wikis. This can confuse
                        // the job runner system into spawning endless RPC requests that fail (T171371).
-                       if ( $wiki !== wfWikiID() && !in_array( $wiki, $wgLocalDatabases ) ) {
+                       if ( !WikiMap::isCurrentWikiId( $wiki ) && !in_array( $wiki, $wgLocalDatabases ) ) {
                                self::$instances[$wiki]->invalidWiki = true;
                        }
                }
@@ -433,7 +433,7 @@ class JobQueueGroup {
         */
        private function getCachedConfigVar( $name ) {
                // @TODO: cleanup this whole method with a proper config system
-               if ( $this->wiki === wfWikiID() ) {
+               if ( WikiMap::isCurrentWikiId( $this->wiki ) ) {
                        return $GLOBALS[$name]; // common case
                } else {
                        $wiki = $this->wiki;
index 223ae32..2d4ce34 100644 (file)
@@ -73,9 +73,8 @@ class RecentChangesUpdateJob extends Job {
        protected function purgeExpiredRows() {
                global $wgRCMaxAge, $wgUpdateRowsPerQuery;
 
-               $lockKey = wfWikiID() . ':recentchanges-prune';
-
                $dbw = wfGetDB( DB_MASTER );
+               $lockKey = $dbw->getDomainID() . ':recentchanges-prune';
                if ( !$dbw->lock( $lockKey, __METHOD__, 0 ) ) {
                        // already in progress
                        return;
@@ -128,7 +127,7 @@ class RecentChangesUpdateJob extends Job {
                $factory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
                $ticket = $factory->getEmptyTransactionTicket( __METHOD__ );
 
-               $lockKey = wfWikiID() . '-activeusers';
+               $lockKey = $dbw->getDomainID() . '-activeusers';
                if ( !$dbw->lock( $lockKey, __METHOD__, 0 ) ) {
                        // Exclusive update (avoids duplicate entries)… it's usually fine to just
                        // drop out here, if the Job is already running.
index 5d7afd3..cdbe1c5 100644 (file)
@@ -82,7 +82,8 @@ class UserMailer {
        static function makeMsgId() {
                global $wgSMTP, $wgServer;
 
-               $msgid = uniqid( wfWikiID() . ".", true ); /* true required for cygwin */
+               $domainId = WikiMap::getCurrentWikiDomain()->getId();
+               $msgid = uniqid( $domainId . ".", true /** for cygwin */ );
                if ( is_array( $wgSMTP ) && isset( $wgSMTP['IDHost'] ) && $wgSMTP['IDHost'] ) {
                        $domain = $wgSMTP['IDHost'];
                } else {
index 081af19..086dc80 100644 (file)
@@ -3481,7 +3481,11 @@ class WikiPage implements Page, IDBAccessObject {
                                // Do not include the namespace since there can be multiple aliases to it
                                // due to different namespace text definitions on different wikis. This only
                                // means that some cache invalidations happen that are not strictly needed.
-                               $cache->makeGlobalKey( 'interwiki-page', wfWikiID(), $title->getDBkey() )
+                               $cache->makeGlobalKey(
+                                       'interwiki-page',
+                                       WikiMap::getCurrentWikiDomain()->getId(),
+                                       $title->getDBkey()
+                               )
                        );
                } );
        }
index f545532..901676a 100644 (file)
@@ -1447,7 +1447,7 @@ abstract class Skin extends ContextSource {
                        return $newMessagesAlert;
                }
 
-               if ( count( $newtalks ) == 1 && $newtalks[0]['wiki'] === wfWikiID() ) {
+               if ( count( $newtalks ) == 1 && WikiMap::isCurrentWikiId( $newtalks[0]['wiki'] ) ) {
                        $uTalkTitle = $user->getTalkPage();
                        $lastSeenRev = $newtalks[0]['rev'] ?? null;
                        $nofAuthors = 0;
index 4205188..00d8d6e 100644 (file)
@@ -497,18 +497,18 @@ class UserrightsPage extends SpecialPage {
                $parts = explode( $this->getConfig()->get( 'UserrightsInterwikiDelimiter' ), $username );
                if ( count( $parts ) < 2 ) {
                        $name = trim( $username );
-                       $database = '';
+                       $wikiId = '';
                } else {
-                       list( $name, $database ) = array_map( 'trim', $parts );
+                       list( $name, $wikiId ) = array_map( 'trim', $parts );
 
-                       if ( $database == wfWikiID() ) {
-                               $database = '';
+                       if ( WikiMap::isCurrentWikiId( $wikiId ) ) {
+                               $wikiId = '';
                        } else {
                                if ( $writing && !$this->getUser()->isAllowed( 'userrights-interwiki' ) ) {
                                        return Status::newFatal( 'userrights-no-interwiki' );
                                }
-                               if ( !UserRightsProxy::validDatabase( $database ) ) {
-                                       return Status::newFatal( 'userrights-nodatabase', $database );
+                               if ( !UserRightsProxy::validDatabase( $wikiId ) ) {
+                                       return Status::newFatal( 'userrights-nodatabase', $wikiId );
                                }
                        }
                }
@@ -522,10 +522,10 @@ class UserrightsPage extends SpecialPage {
                        // We'll do a lookup for the name internally.
                        $id = intval( substr( $name, 1 ) );
 
-                       if ( $database == '' ) {
+                       if ( $wikiId == '' ) {
                                $name = User::whoIs( $id );
                        } else {
-                               $name = UserRightsProxy::whoIs( $database, $id );
+                               $name = UserRightsProxy::whoIs( $wikiId, $id );
                        }
 
                        if ( !$name ) {
@@ -539,10 +539,10 @@ class UserrightsPage extends SpecialPage {
                        }
                }
 
-               if ( $database == '' ) {
+               if ( $wikiId == '' ) {
                        $user = User::newFromName( $name );
                } else {
-                       $user = UserRightsProxy::newFromName( $database, $name );
+                       $user = UserRightsProxy::newFromName( $wikiId, $name );
                }
 
                if ( !$user || $user->isAnon() ) {
index 0a34554..ca3db5b 100644 (file)
@@ -41,7 +41,7 @@ class LocalIdLookup extends CentralIdLookup {
                }
 
                // Easy case, we're checking locally
-               if ( $wikiId === null || $wikiId === wfWikiID() ) {
+               if ( $wikiId === null || WikiMap::isCurrentWikiId( $wikiId ) ) {
                        return true;
                }
 
index 5e5ca1b..8cbedb9 100644 (file)
@@ -2638,7 +2638,7 @@ class User implements IDBAccessObject, UserIdentity {
                        // and it is always for the same wiki, but we double-check here in
                        // case that changes some time in the future.
                        if ( count( $newMessageLinks ) === 1
-                               && $newMessageLinks[0]['wiki'] === wfWikiID()
+                               && WikiMap::isCurrentWikiId( $newMessageLinks[0]['wiki'] )
                                && $newMessageLinks[0]['rev']
                        ) {
                                /** @var Revision $newMessageRevision */
index 3c2731a..0b7a720 100644 (file)
@@ -27,53 +27,54 @@ use Wikimedia\Rdbms\IDatabase;
  * user rights manipulation.
  */
 class UserRightsProxy {
+       /** @var IDatabase */
+       private $db;
+       /** @var string */
+       private $wikiId;
+       /** @var string */
+       private $name;
+       /** @var int */
+       private $id;
+       /** @var array */
+       private $newOptions;
 
        /**
         * @see newFromId()
         * @see newFromName()
         * @param IDatabase $db Db connection
-        * @param string $database Database name
+        * @param string $wikiId Database name
         * @param string $name User name
         * @param int $id User ID
         */
-       private function __construct( $db, $database, $name, $id ) {
+       private function __construct( $db, $wikiId, $name, $id ) {
                $this->db = $db;
-               $this->database = $database;
+               $this->wikiId = $wikiId;
                $this->name = $name;
                $this->id = intval( $id );
                $this->newOptions = [];
        }
 
-       /**
-        * Accessor for $this->database
-        *
-        * @return string Database name
-        */
-       public function getDBName() {
-               return $this->database;
-       }
-
        /**
         * Confirm the selected database name is a valid local interwiki database name.
         *
-        * @param string $database Database name
+        * @param string $wikiId Database name
         * @return bool
         */
-       public static function validDatabase( $database ) {
+       public static function validDatabase( $wikiId ) {
                global $wgLocalDatabases;
-               return in_array( $database, $wgLocalDatabases );
+               return in_array( $wikiId, $wgLocalDatabases );
        }
 
        /**
         * Same as User::whoIs()
         *
-        * @param string $database Database name
+        * @param string $wikiId Database name
         * @param int $id User ID
-        * @param bool $ignoreInvalidDB If true, don't check if $database is in $wgLocalDatabases
+        * @param bool $ignoreInvalidDB If true, don't check if $wikiId is in $wgLocalDatabases
         * @return string User name or false if the user doesn't exist
         */
-       public static function whoIs( $database, $id, $ignoreInvalidDB = false ) {
-               $user = self::newFromId( $database, $id, $ignoreInvalidDB );
+       public static function whoIs( $wikiId, $id, $ignoreInvalidDB = false ) {
+               $user = self::newFromId( $wikiId, $id, $ignoreInvalidDB );
                if ( $user ) {
                        return $user->name;
                } else {
@@ -84,35 +85,35 @@ class UserRightsProxy {
        /**
         * Factory function; get a remote user entry by ID number.
         *
-        * @param string $database Database name
+        * @param string $wikiId Database name
         * @param int $id User ID
-        * @param bool $ignoreInvalidDB If true, don't check if $database is in $wgLocalDatabases
+        * @param bool $ignoreInvalidDB If true, don't check if $wikiId is in $wgLocalDatabases
         * @return UserRightsProxy|null If doesn't exist
         */
-       public static function newFromId( $database, $id, $ignoreInvalidDB = false ) {
-               return self::newFromLookup( $database, 'user_id', intval( $id ), $ignoreInvalidDB );
+       public static function newFromId( $wikiId, $id, $ignoreInvalidDB = false ) {
+               return self::newFromLookup( $wikiId, 'user_id', intval( $id ), $ignoreInvalidDB );
        }
 
        /**
         * Factory function; get a remote user entry by name.
         *
-        * @param string $database Database name
+        * @param string $wikiId Database name
         * @param string $name User name
-        * @param bool $ignoreInvalidDB If true, don't check if $database is in $wgLocalDatabases
+        * @param bool $ignoreInvalidDB If true, don't check if $wikiId is in $wgLocalDatabases
         * @return UserRightsProxy|null If doesn't exist
         */
-       public static function newFromName( $database, $name, $ignoreInvalidDB = false ) {
-               return self::newFromLookup( $database, 'user_name', $name, $ignoreInvalidDB );
+       public static function newFromName( $wikiId, $name, $ignoreInvalidDB = false ) {
+               return self::newFromLookup( $wikiId, 'user_name', $name, $ignoreInvalidDB );
        }
 
        /**
-        * @param string $database
+        * @param string $wikiId
         * @param string $field
         * @param string $value
         * @param bool $ignoreInvalidDB
         * @return null|UserRightsProxy
         */
-       private static function newFromLookup( $database, $field, $value, $ignoreInvalidDB = false ) {
+       private static function newFromLookup( $wikiId, $field, $value, $ignoreInvalidDB = false ) {
                global $wgSharedDB, $wgSharedTables;
                // If the user table is shared, perform the user query on it,
                // but don't pass it to the UserRightsProxy,
@@ -120,10 +121,10 @@ class UserRightsProxy {
                if ( $wgSharedDB && in_array( 'user', $wgSharedTables ) ) {
                        $userdb = self::getDB( $wgSharedDB, $ignoreInvalidDB );
                } else {
-                       $userdb = self::getDB( $database, $ignoreInvalidDB );
+                       $userdb = self::getDB( $wikiId, $ignoreInvalidDB );
                }
 
-               $db = self::getDB( $database, $ignoreInvalidDB );
+               $db = self::getDB( $wikiId, $ignoreInvalidDB );
 
                if ( $db && $userdb ) {
                        $row = $userdb->selectRow( 'user',
@@ -132,9 +133,8 @@ class UserRightsProxy {
                                __METHOD__ );
 
                        if ( $row !== false ) {
-                               return new UserRightsProxy( $db, $database,
-                                       $row->user_name,
-                                       intval( $row->user_id ) );
+                               return new UserRightsProxy(
+                                       $db, $wikiId, $row->user_name, intval( $row->user_id ) );
                        }
                }
                return null;
@@ -144,18 +144,17 @@ class UserRightsProxy {
         * Open a database connection to work on for the requested user.
         * This may be a new connection to another database for remote users.
         *
-        * @param string $database
-        * @param bool $ignoreInvalidDB If true, don't check if $database is in $wgLocalDatabases
+        * @param string $wikiId
+        * @param bool $ignoreInvalidDB If true, don't check if $wikiId is in $wgLocalDatabases
         * @return IDatabase|null If invalid selection
         */
-       public static function getDB( $database, $ignoreInvalidDB = false ) {
-               global $wgDBname;
-               if ( $ignoreInvalidDB || self::validDatabase( $database ) ) {
-                       if ( $database == $wgDBname ) {
+       public static function getDB( $wikiId, $ignoreInvalidDB = false ) {
+               if ( $ignoreInvalidDB || self::validDatabase( $wikiId ) ) {
+                       if ( WikiMap::isCurrentWikiId( $wikiId ) ) {
                                // Hmm... this shouldn't happen though. :)
                                return wfGetDB( DB_MASTER );
                        } else {
-                               return wfGetDB( DB_MASTER, [], $database );
+                               return wfGetDB( DB_MASTER, [], $wikiId );
                        }
                }
                return null;
@@ -181,7 +180,7 @@ class UserRightsProxy {
         * @return string
         */
        public function getName() {
-               return $this->name . '@' . $this->database;
+               return $this->name . '@' . $this->wikiId;
        }
 
        /**
index 53e0b10..199f638 100644 (file)
@@ -235,6 +235,14 @@ class WikiMapTest extends MediaWikiLangTestCase {
                $this->assertEquals( $wiki, WikiMap::getWikiFromUrl( $url ) );
        }
 
+       public function provideGetWikiIdFromDomain() {
+               return [
+                       [ 'db-prefix', 'db-prefix' ],
+                       [ wfWikiID(), wfWikiID() ],
+                       [ new DatabaseDomain( 'db-dash', null, 'prefix' ), 'db-dash-prefix' ]
+               ];
+       }
+
        /**
         * @dataProvider provideGetWikiIdFromDomain
         * @covers WikiMap::getWikiIdFromDomain()
@@ -243,11 +251,48 @@ class WikiMapTest extends MediaWikiLangTestCase {
                $this->assertEquals( $wikiId, WikiMap::getWikiIdFromDomain( $domain ) );
        }
 
-       public function provideGetWikiIdFromDomain() {
+       /**
+        * @covers WikiMap::isCurrentWikiDomain()
+        * @covers WikiMap::getCurrentWikiDomain()
+        */
+       public function testIsCurrentWikiDomain() {
+               $this->assertTrue( WikiMap::isCurrentWikiDomain( wfWikiID() ) );
+
+               $localDomain = DatabaseDomain::newFromId( wfWikiID() );
+               $domain1 = new DatabaseDomain(
+                       $localDomain->getDatabase(), 'someschema', $localDomain->getTablePrefix() );
+               $domain2 = new DatabaseDomain(
+                       $localDomain->getDatabase(), null, $localDomain->getTablePrefix() );
+
+               $this->assertTrue( WikiMap::isCurrentWikiDomain( $domain1 ), 'Schema ignored' );
+               $this->assertTrue( WikiMap::isCurrentWikiDomain( $domain2 ), 'Schema ignored' );
+
+               $this->assertTrue( WikiMap::isCurrentWikiDomain( WikiMap::getCurrentWikiDomain() ) );
+       }
+
+       public function provideIsCurrentWikiId() {
                return [
-                       [ 'db-prefix', 'db-prefix' ],
-                       [ wfWikiID(), wfWikiID() ],
-                       [ new DatabaseDomain( 'db-dash', null, 'prefix' ), 'db-dash-prefix' ]
+                       [ 'db', 'db', null, '' ],
+                       [ 'db','db', 'schema', '' ],
+                       [ 'db-prefix', 'db', null, 'prefix' ],
+                       [ 'db-prefix', 'db', 'schema', 'prefix' ],
+                       // Bad hyphen cases (best effort support)
+                       [ 'db-stuff', 'db-stuff', null, '' ],
+                       [ 'db-stuff-prefix', 'db-stuff', null, 'prefix' ]
                ];
        }
+
+       /**
+        * @dataProvider provideIsCurrentWikiId
+        * @covers WikiMap::isCurrentWikiId()
+        * @covers WikiMap::getCurrentWikiDomain()
+        * @covers WikiMap::getWikiIdFromDomain()
+        */
+       public function testIsCurrentWikiId( $wikiId, $db, $schema, $prefix ) {
+               $this->setMwGlobals(
+                       [ 'wgDBname' => $db, 'wgDBmwschema' => $schema, 'wgDBprefix' => $prefix ] );
+
+               $this->assertTrue( WikiMap::isCurrentWikiId( $wikiId ), "ID matches" );
+               $this->assertNotTrue( WikiMap::isCurrentWikiId( $wikiId . '-more' ), "Bogus ID" );
+       }
 }