From e51f95dea3df42b95c0e7e3e5e336dff64eb1d3e Mon Sep 17 00:00:00 2001 From: addshore Date: Sat, 23 Dec 2017 17:14:28 +0000 Subject: [PATCH] [MCR] Introduce BlobStoreFactory This allows Revision::getRevisionText to get a different BlobStore instance when $wiki is passed in restoring the behaviour for $wiki before the MCR Revision overhaul patch was merged. Ia4c20a91e98df0b9b14b138eb4825c55e5200384 Bug: T183634 Bug: T183631 bug: T183583 Change-Id: Ib0949454e9a003c2965adc1aab38e31fcf121afe --- autoload.php | 1 + includes/MediaWikiServices.php | 11 +- includes/Revision.php | 10 +- includes/ServiceWiring.php | 29 ++--- includes/Storage/BlobStoreFactory.php | 105 ++++++++++++++++++ includes/Storage/SqlBlobStore.php | 2 +- .../includes/MediaWikiServicesTest.php | 2 + tests/phpunit/includes/RevisionTest.php | 27 ++++- .../includes/Storage/BlobStoreFactoryTest.php | 46 ++++++++ 9 files changed, 205 insertions(+), 28 deletions(-) create mode 100644 includes/Storage/BlobStoreFactory.php create mode 100644 tests/phpunit/includes/Storage/BlobStoreFactoryTest.php diff --git a/autoload.php b/autoload.php index 6b8387b4f7..c37d9f71f3 100644 --- a/autoload.php +++ b/autoload.php @@ -944,6 +944,7 @@ $wgAutoloadLocalClasses = [ 'MediaWiki\\Site\\MediaWikiPageNameNormalizer' => __DIR__ . '/includes/site/MediaWikiPageNameNormalizer.php', 'MediaWiki\\Storage\\BlobAccessException' => __DIR__ . '/includes/Storage/BlobAccessException.php', 'MediaWiki\\Storage\\BlobStore' => __DIR__ . '/includes/Storage/BlobStore.php', + 'MediaWiki\\Storage\\BlobStoreFactory' => __DIR__ . '/includes/Storage/BlobStoreFactory.php', 'MediaWiki\\Storage\\IncompleteRevisionException' => __DIR__ . '/includes/Storage/IncompleteRevisionException.php', 'MediaWiki\\Storage\\MutableRevisionRecord' => __DIR__ . '/includes/Storage/MutableRevisionRecord.php', 'MediaWiki\\Storage\\MutableRevisionSlots' => __DIR__ . '/includes/Storage/MutableRevisionSlots.php', diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php index 33d0fd4d3d..04c67fb297 100644 --- a/includes/MediaWikiServices.php +++ b/includes/MediaWikiServices.php @@ -12,6 +12,7 @@ use Hooks; use IBufferingStatsdDataFactory; use MediaWiki\Shell\CommandFactory; use MediaWiki\Storage\BlobStore; +use MediaWiki\Storage\BlobStoreFactory; use MediaWiki\Storage\RevisionStore; use Wikimedia\Rdbms\LBFactory; use LinkCache; @@ -700,12 +701,20 @@ class MediaWikiServices extends ServiceContainer { return $this->getService( 'ExternalStoreFactory' ); } + /** + * @since 1.31 + * @return BlobStoreFactory + */ + public function getBlobStoreFactory() { + return $this->getService( 'BlobStoreFactory' ); + } + /** * @since 1.31 * @return BlobStore */ public function getBlobStore() { - return $this->getService( 'BlobStore' ); + return $this->getService( '_SqlBlobStore' ); } /** diff --git a/includes/Revision.php b/includes/Revision.php index ed0646afd1..af4fef51de 100644 --- a/includes/Revision.php +++ b/includes/Revision.php @@ -65,10 +65,14 @@ class Revision implements IDBAccessObject { } /** + * @param bool|string $wikiId The ID of the target wiki database. Use false for the local wiki. + * * @return SqlBlobStore */ - protected static function getBlobStore() { - $store = MediaWikiServices::getInstance()->getBlobStore(); + protected static function getBlobStore( $wiki = false ) { + $store = MediaWikiServices::getInstance() + ->getBlobStoreFactory() + ->newSqlBlobStore( $wiki ); if ( !$store instanceof SqlBlobStore ) { throw new RuntimeException( @@ -942,7 +946,7 @@ class Revision implements IDBAccessObject { $cacheKey = isset( $row->old_id ) ? ( 'tt:' . $row->old_id ) : null; - return self::getBlobStore()->expandBlob( $text, $flags, $cacheKey ); + return self::getBlobStore( $wiki )->expandBlob( $text, $flags, $cacheKey ); } /** diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index d21bcef332..0d266fb710 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -42,6 +42,7 @@ use MediaWiki\Linker\LinkRendererFactory; use MediaWiki\Logger\LoggerFactory; use MediaWiki\MediaWikiServices; use MediaWiki\Shell\CommandFactory; +use MediaWiki\Storage\BlobStoreFactory; use MediaWiki\Storage\RevisionStore; use MediaWiki\Storage\SqlBlobStore; @@ -474,28 +475,22 @@ return [ return $store; }, + 'BlobStoreFactory' => function ( MediaWikiServices $services ) { + global $wgContLang; + return new BlobStoreFactory( + $services->getDBLoadBalancer(), + $services->getMainWANObjectCache(), + $services->getMainConfig(), + $wgContLang + ); + }, + 'BlobStore' => function ( MediaWikiServices $services ) { return $services->getService( '_SqlBlobStore' ); }, '_SqlBlobStore' => function ( MediaWikiServices $services ) { - global $wgContLang; // TODO: manage $wgContLang as a service - - $store = new SqlBlobStore( - $services->getDBLoadBalancer(), - $services->getMainWANObjectCache() - ); - - $config = $services->getMainConfig(); - $store->setCompressBlobs( $config->get( 'CompressRevisions' ) ); - $store->setCacheExpiry( $config->get( 'RevisionCacheExpiry' ) ); - $store->setUseExternalStore( $config->get( 'DefaultExternalStore' ) !== false ); - - if ( $config->get( 'LegacyEncoding' ) ) { - $store->setLegacyEncoding( $config->get( 'LegacyEncoding' ), $wgContLang ); - } - - return $store; + return $services->getBlobStoreFactory()->newSqlBlobStore(); }, /////////////////////////////////////////////////////////////////////////// diff --git a/includes/Storage/BlobStoreFactory.php b/includes/Storage/BlobStoreFactory.php new file mode 100644 index 0000000000..63ca74def4 --- /dev/null +++ b/includes/Storage/BlobStoreFactory.php @@ -0,0 +1,105 @@ +loadBalancer = $loadBalancer; + $this->cache = $cache; + $this->config = $mainConfig; + $this->contLang = $contLang; + } + + /** + * @since 1.31 + * + * @param bool|string $wikiId The ID of the target wiki database. Use false for the local wiki. + * + * @return BlobStore + */ + public function newBlobStore( $wikiId = false ) { + return $this->newSqlBlobStore( $wikiId ); + } + + /** + * @internal Please call newBlobStore and use the BlobStore interface. + * + * @param bool|string $wikiId The ID of the target wiki database. Use false for the local wiki. + * + * @return SqlBlobStore + */ + public function newSqlBlobStore( $wikiId = false ) { + $store = new SqlBlobStore( + $this->loadBalancer, + $this->cache, + $wikiId + ); + + $store->setCompressBlobs( $this->config->get( 'CompressRevisions' ) ); + $store->setCacheExpiry( $this->config->get( 'RevisionCacheExpiry' ) ); + $store->setUseExternalStore( $this->config->get( 'DefaultExternalStore' ) !== false ); + + if ( $this->config->get( 'LegacyEncoding' ) ) { + $store->setLegacyEncoding( $this->config->get( 'LegacyEncoding' ), $this->contLang ); + } + + return $store; + } + +} diff --git a/includes/Storage/SqlBlobStore.php b/includes/Storage/SqlBlobStore.php index fcdc1b908c..69e1539ad1 100644 --- a/includes/Storage/SqlBlobStore.php +++ b/includes/Storage/SqlBlobStore.php @@ -382,7 +382,7 @@ class SqlBlobStore implements IDBAccessObject, BlobStore { return false; } - if ( $cacheKey ) { + if ( $cacheKey && $this->wikiId === false ) { // Make use of the wiki-local revision text cache. // The cached value should be decompressed, so handle that and return here. // NOTE: we rely on $this->cache being the right cache for $this->wikiId! diff --git a/tests/phpunit/includes/MediaWikiServicesTest.php b/tests/phpunit/includes/MediaWikiServicesTest.php index 6bab16feeb..dbb7799b55 100644 --- a/tests/phpunit/includes/MediaWikiServicesTest.php +++ b/tests/phpunit/includes/MediaWikiServicesTest.php @@ -8,6 +8,7 @@ use MediaWiki\Services\SalvageableService; use MediaWiki\Services\ServiceDisabledException; use MediaWiki\Shell\CommandFactory; use MediaWiki\Storage\BlobStore; +use MediaWiki\Storage\BlobStoreFactory; use MediaWiki\Storage\RevisionStore; use MediaWiki\Storage\SqlBlobStore; @@ -334,6 +335,7 @@ class MediaWikiServicesTest extends MediaWikiTestCase { 'LocalServerObjectCache' => [ 'LocalServerObjectCache', BagOStuff::class ], 'VirtualRESTServiceClient' => [ 'VirtualRESTServiceClient', VirtualRESTServiceClient::class ], 'ShellCommandFactory' => [ 'ShellCommandFactory', CommandFactory::class ], + 'BlobStoreFactory' => [ 'BlobStoreFactory', BlobStoreFactory::class ], 'BlobStore' => [ 'BlobStore', BlobStore::class ], '_SqlBlobStore' => [ '_SqlBlobStore', SqlBlobStore::class ], 'RevisionStore' => [ 'RevisionStore', RevisionStore::class ], diff --git a/tests/phpunit/includes/RevisionTest.php b/tests/phpunit/includes/RevisionTest.php index 566dc9285a..80257cca8b 100644 --- a/tests/phpunit/includes/RevisionTest.php +++ b/tests/phpunit/includes/RevisionTest.php @@ -1,5 +1,6 @@ setService( '_SqlBlobStore', $blobStore ); + $this->setService( 'BlobStoreFactory', $this->mockBlobStoreFactory( $blobStore ) ); $row = (object)$arrayData; $rev = new Revision( $row, 0, $this->getMockTitle() ); @@ -435,6 +436,20 @@ class RevisionTest extends MediaWikiTestCase { return $blobStore; } + private function mockBlobStoreFactory( $blobStore ) { + /** @var LoadBalancer $lb */ + $factory = $this->getMockBuilder( BlobStoreFactory::class ) + ->disableOriginalConstructor() + ->getMock(); + $factory->expects( $this->any() ) + ->method( 'newBlobStore' ) + ->willReturn( $blobStore ); + $factory->expects( $this->any() ) + ->method( 'newSqlBlobStore' ) + ->willReturn( $blobStore ); + return $factory; + } + /** * @return RevisionStore */ @@ -478,7 +493,7 @@ class RevisionTest extends MediaWikiTestCase { public function testGetRevisionWithLegacyEncoding( $expected, $lang, $encoding, $rowData ) { $blobStore = $this->getBlobStore(); $blobStore->setLegacyEncoding( $encoding, Language::factory( $lang ) ); - $this->setService( 'BlobStore', $blobStore ); + $this->setService( 'BlobStoreFactory', $this->mockBlobStoreFactory( $blobStore ) ); $this->testGetRevisionText( $expected, $rowData ); } @@ -518,7 +533,7 @@ class RevisionTest extends MediaWikiTestCase { $blobStore = $this->getBlobStore(); $blobStore->setLegacyEncoding( $encoding, Language::factory( $lang ) ); - $this->setService( 'BlobStore', $blobStore ); + $this->setService( 'BlobStoreFactory', $this->mockBlobStoreFactory( $blobStore ) ); $this->testGetRevisionText( $expected, $rowData ); } @@ -548,7 +563,7 @@ class RevisionTest extends MediaWikiTestCase { $blobStore = $this->getBlobStore(); $blobStore->setCompressBlobs( true ); - $this->setService( 'BlobStore', $blobStore ); + $this->setService( 'BlobStoreFactory', $this->mockBlobStoreFactory( $blobStore ) ); $row = new stdClass; $row->old_text = "Wiki est l'\xc3\xa9cole superieur !"; @@ -693,7 +708,7 @@ class RevisionTest extends MediaWikiTestCase { $blobStore->setLegacyEncoding( $legacyEncoding, Language::factory( 'en' ) ); } - $this->setService( 'BlobStore', $blobStore ); + $this->setService( 'BlobStoreFactory', $this->mockBlobStoreFactory( $blobStore ) ); $this->assertSame( $expected, Revision::decompressRevisionText( $text, $flags ) @@ -802,7 +817,7 @@ class RevisionTest extends MediaWikiTestCase { ->getMock(); $blobStore = new SqlBlobStore( $lb, $cache ); - $this->setService( 'BlobStore', $blobStore ); + $this->setService( 'BlobStoreFactory', $this->mockBlobStoreFactory( $blobStore ) ); $this->assertSame( 'AAAABBAAA', diff --git a/tests/phpunit/includes/Storage/BlobStoreFactoryTest.php b/tests/phpunit/includes/Storage/BlobStoreFactoryTest.php new file mode 100644 index 0000000000..46ba7a5da1 --- /dev/null +++ b/tests/phpunit/includes/Storage/BlobStoreFactoryTest.php @@ -0,0 +1,46 @@ +getBlobStoreFactory(); + $store = $factory->newBlobStore( $wikiId ); + $this->assertInstanceOf( BlobStore::class, $store ); + + // This only works as we currently know this is a SqlBlobStore object + $wrapper = TestingAccessWrapper::newFromObject( $store ); + $this->assertEquals( $wikiId, $wrapper->wikiId ); + } + + /** + * @dataProvider provideWikiIds + */ + public function testNewSqlBlobStore( $wikiId ) { + $factory = MediaWikiServices::getInstance()->getBlobStoreFactory(); + $store = $factory->newSqlBlobStore( $wikiId ); + $this->assertInstanceOf( SqlBlobStore::class, $store ); + + $wrapper = TestingAccessWrapper::newFromObject( $store ); + $this->assertEquals( $wikiId, $wrapper->wikiId ); + } + +} -- 2.20.1