From 7dbd37a7c773200b71ddece7a292c7e86efdd52b Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Thu, 28 Mar 2019 15:37:52 -0700 Subject: [PATCH] Make WikiMap::isCurrentWikiDbDomain() more rigorous This already requires a DB domain ID, so there is no reason to have hacks for trying to handle a wiki ID being passed in instead. If the provided domain has a schema, it should not simply be ignored in the comparison. Change-Id: I9ced7a46fa05f32843a9a7d17391c5d0576b099c --- includes/WikiMap.php | 40 ++++++++----------- includes/jobqueue/JobQueue.php | 2 +- tests/phpunit/includes/WikiMapTest.php | 11 +++-- .../includes/jobqueue/JobQueueMemoryTest.php | 2 +- .../includes/jobqueue/JobQueueTest.php | 7 +++- 5 files changed, 30 insertions(+), 32 deletions(-) diff --git a/includes/WikiMap.php b/includes/WikiMap.php index dbad4b0741..8b000f2063 100644 --- a/includes/WikiMap.php +++ b/includes/WikiMap.php @@ -245,8 +245,12 @@ class WikiMap { /** * Get the wiki ID of a database domain * - * This is like DatabaseDomain::getId() without encoding (for legacy reasons) - * and without the schema if it merely set to the generic value "mediawiki" + * This is like DatabaseDomain::getId() without encoding (for legacy reasons) and + * without the schema if it is the generic installer default of "mediawiki"/"dbo" + * + * @see $wgDBmwschema + * @see PostgresInstaller + * @see MssqlInstaller * * @param string|DatabaseDomain $domain * @return string @@ -254,15 +258,17 @@ class WikiMap { */ public static function getWikiIdFromDbDomain( $domain ) { $domain = DatabaseDomain::newFromId( $domain ); - + // Since the schema was not always part of the wiki ID, try to maintain backwards + // compatibility with some common cases. Assume that if the DB domain schema is just + // the installer default then it is probably the case that the schema is the same for + // all wikis in the farm. Historically, any wiki farm had to make the database/prefix + // combination unique per wiki. Ommit the schema if it does not seem wiki specific. if ( !in_array( $domain->getSchema(), [ null, 'mediawiki', 'dbo' ], true ) ) { - // Include the schema if it is set and is not the default placeholder. // This means a site admin may have specifically taylored the schemas. - // Domain IDs might use the form --, meaning that - // the schema portion must be accounted for to disambiguate wikis. + // Domain IDs might use the form -- or --_, + // meaning that the schema portion must be accounted for to disambiguate wikis. return "{$domain->getDatabase()}-{$domain->getSchema()}-{$domain->getTablePrefix()}"; } - // Note that if this wiki ID is passed a a domain ID to LoadBalancer, then it can // handle the schema by assuming the generic "mediawiki" schema if needed. return strlen( $domain->getTablePrefix() ) @@ -291,30 +297,16 @@ class WikiMap { /** * @param DatabaseDomain|string $domain - * @return bool Whether $domain has the same DB/prefix as the current wiki + * @return bool Whether $domain matches the DB domain of the current wiki * @since 1.33 */ public static function isCurrentWikiDbDomain( $domain ) { - $domain = DatabaseDomain::newFromId( $domain ); - $curDomain = self::getCurrentWikiDbDomain(); - - if ( !in_array( $curDomain->getSchema(), [ null, 'mediawiki', 'dbo' ], true ) ) { - // Include the schema if it is set and is not the default placeholder. - // This means a site admin may have specifically taylored the schemas. - // Domain IDs might use the form --, meaning that - // the schema portion must be accounted for to disambiguate wikis. - return $curDomain->equals( $domain ); - } - - return ( - $curDomain->getDatabase() === $domain->getDatabase() && - $curDomain->getTablePrefix() === $domain->getTablePrefix() - ); + return self::getCurrentWikiDbDomain()->equals( DatabaseDomain::newFromId( $domain ) ); } /** * @param string $wikiId - * @return bool Whether $wikiId has the same DB/prefix as the current wiki + * @return bool Whether $wikiId matches the wiki ID of the current wiki * @since 1.33 */ public static function isCurrentWikiId( $wikiId ) { diff --git a/includes/jobqueue/JobQueue.php b/includes/jobqueue/JobQueue.php index 660352ac79..fdcd65cc9b 100644 --- a/includes/jobqueue/JobQueue.php +++ b/includes/jobqueue/JobQueue.php @@ -126,7 +126,7 @@ abstract class JobQueue { * @deprecated 1.33 */ final public function getWiki() { - return $this->domain; + return WikiMap::getWikiIdFromDbDomain( $this->domain ); } /** diff --git a/tests/phpunit/includes/WikiMapTest.php b/tests/phpunit/includes/WikiMapTest.php index df05671dd2..1608b9c955 100644 --- a/tests/phpunit/includes/WikiMapTest.php +++ b/tests/phpunit/includes/WikiMapTest.php @@ -260,16 +260,19 @@ class WikiMapTest extends MediaWikiLangTestCase { * @covers WikiMap::getCurrentWikiDbDomain() */ public function testIsCurrentWikiDomain() { - $this->assertTrue( WikiMap::isCurrentWikiDbDomain( wfWikiID() ) ); + $this->setMwGlobals( 'wgDBmwschema', 'mediawiki' ); - $localDomain = DatabaseDomain::newFromId( wfWikiID() ); + $localDomain = WikiMap::getCurrentWikiDbDomain()->getId(); + $this->assertTrue( WikiMap::isCurrentWikiDbDomain( $localDomain ) ); + + $localDomain = DatabaseDomain::newFromId( $localDomain ); $domain1 = new DatabaseDomain( $localDomain->getDatabase(), 'someschema', $localDomain->getTablePrefix() ); $domain2 = new DatabaseDomain( $localDomain->getDatabase(), null, $localDomain->getTablePrefix() ); - $this->assertTrue( WikiMap::isCurrentWikiDbDomain( $domain1 ), 'Schema ignored' ); - $this->assertTrue( WikiMap::isCurrentWikiDbDomain( $domain2 ), 'Schema ignored' ); + $this->assertFalse( WikiMap::isCurrentWikiDbDomain( $domain1 ), 'Schema not ignored' ); + $this->assertFalse( WikiMap::isCurrentWikiDbDomain( $domain2 ), 'Null schema not ignored' ); $this->assertTrue( WikiMap::isCurrentWikiDbDomain( WikiMap::getCurrentWikiDbDomain() ) ); } diff --git a/tests/phpunit/includes/jobqueue/JobQueueMemoryTest.php b/tests/phpunit/includes/jobqueue/JobQueueMemoryTest.php index b2e7ea4e55..232b46a31b 100644 --- a/tests/phpunit/includes/jobqueue/JobQueueMemoryTest.php +++ b/tests/phpunit/includes/jobqueue/JobQueueMemoryTest.php @@ -18,7 +18,7 @@ class JobQueueMemoryTest extends PHPUnit\Framework\TestCase { private function newJobQueue() { return JobQueue::factory( [ 'class' => JobQueueMemory::class, - 'wiki' => wfWikiID(), + 'domain' => WikiMap::getCurrentWikiDbDomain()->getId(), 'type' => 'null', ] ); } diff --git a/tests/phpunit/includes/jobqueue/JobQueueTest.php b/tests/phpunit/includes/jobqueue/JobQueueTest.php index 0421fe7c68..d38c6c7093 100644 --- a/tests/phpunit/includes/jobqueue/JobQueueTest.php +++ b/tests/phpunit/includes/jobqueue/JobQueueTest.php @@ -31,7 +31,7 @@ class JobQueueTest extends MediaWikiTestCase { $baseConfig = [ 'class' => JobQueueDBSingle::class ]; } $baseConfig['type'] = 'null'; - $baseConfig['wiki'] = wfWikiID(); + $baseConfig['domain'] = WikiMap::getCurrentWikiDbDomain()->getId(); $variants = [ 'queueRand' => [ 'order' => 'random', 'claimTTL' => 0 ], 'queueRandTTL' => [ 'order' => 'random', 'claimTTL' => 10 ], @@ -75,7 +75,10 @@ class JobQueueTest extends MediaWikiTestCase { $this->markTestSkipped( $desc ); } $this->assertEquals( wfWikiID(), $queue->getWiki(), "Proper wiki ID ($desc)" ); - $this->assertEquals( wfWikiID(), $queue->getDomain(), "Proper wiki ID ($desc)" ); + $this->assertEquals( + WikiMap::getCurrentWikiDbDomain()->getId(), + $queue->getDomain(), + "Proper wiki ID ($desc)" ); } /** -- 2.20.1