From f5cc67524df62f7182f54f64e6990238868cde1a Mon Sep 17 00:00:00 2001 From: addshore Date: Wed, 27 Jun 2018 13:16:35 +0100 Subject: [PATCH] Introduce RevisionStoreFactory & Tests This is based on I0a8a441b803, which was reverted because it was incomplete. Bug: T198701 Change-Id: I3e4a5f1ef687418c06dfc979cfe04da336e876b1 --- includes/MediaWikiServices.php | 9 + includes/ServiceWiring.php | 28 ++- includes/Storage/NameTableStore.php | 5 +- includes/Storage/RevisionStore.php | 10 +- includes/Storage/RevisionStoreFactory.php | 168 ++++++++++++++++++ .../includes/MediaWikiServicesTest.php | 2 + .../Storage/RevisionStoreFactoryTest.php | 165 +++++++++++++++++ 7 files changed, 364 insertions(+), 23 deletions(-) create mode 100644 includes/Storage/RevisionStoreFactory.php create mode 100644 tests/phpunit/includes/Storage/RevisionStoreFactoryTest.php diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php index 472a453f6d..fd9b472f96 100644 --- a/includes/MediaWikiServices.php +++ b/includes/MediaWikiServices.php @@ -22,6 +22,7 @@ use MediaWiki\Storage\RevisionFactory; use MediaWiki\Storage\RevisionLookup; use MediaWiki\Storage\RevisionStore; use OldRevisionImporter; +use MediaWiki\Storage\RevisionStoreFactory; use UploadRevisionImporter; use Wikimedia\Rdbms\LBFactory; use LinkCache; @@ -770,6 +771,14 @@ class MediaWikiServices extends ServiceContainer { return $this->getService( 'RevisionStore' ); } + /** + * @since 1.32 + * @return RevisionStoreFactory + */ + public function getRevisionStoreFactory() { + return $this->getService( 'RevisionStoreFactory' ); + } + /** * @since 1.31 * @return RevisionLookup diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index 853b06deba..c33dc94094 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -47,8 +47,7 @@ use MediaWiki\Preferences\DefaultPreferencesFactory; use MediaWiki\Shell\CommandFactory; use MediaWiki\Storage\BlobStoreFactory; use MediaWiki\Storage\NameTableStore; -use MediaWiki\Storage\RevisionStore; -use MediaWiki\Storage\SqlBlobStore; +use MediaWiki\Storage\RevisionStoreFactory; use Wikimedia\ObjectFactory; return [ @@ -474,25 +473,22 @@ return [ }, 'RevisionStore' => function ( MediaWikiServices $services ) { - /** @var SqlBlobStore $blobStore */ - $blobStore = $services->getService( '_SqlBlobStore' ); + return $services->getRevisionStoreFactory()->getRevisionStore(); + }, - $store = new RevisionStore( - $services->getDBLoadBalancer(), - $blobStore, + 'RevisionStoreFactory' => function ( MediaWikiServices $services ) { + $config = $services->getMainConfig(); + $store = new RevisionStoreFactory( + $services->getDBLoadBalancerFactory(), + $services->getBlobStoreFactory(), $services->getMainWANObjectCache(), $services->getCommentStore(), - $services->getContentModelStore(), - $services->getSlotRoleStore(), - $services->getMainConfig()->get( 'MultiContentRevisionSchemaMigrationStage' ), - $services->getActorMigration() + $services->getActorMigration(), + $config->get( 'MultiContentRevisionSchemaMigrationStage' ), + LoggerFactory::getProvider(), + $config->get( 'ContentHandlerUseDB' ) ); - $store->setLogger( LoggerFactory::getInstance( 'RevisionStore' ) ); - - $config = $services->getMainConfig(); - $store->setContentHandlerUseDB( $config->get( 'ContentHandlerUseDB' ) ); - return $store; }, diff --git a/includes/Storage/NameTableStore.php b/includes/Storage/NameTableStore.php index 3516ffe664..23b2902534 100644 --- a/includes/Storage/NameTableStore.php +++ b/includes/Storage/NameTableStore.php @@ -26,6 +26,7 @@ use WANObjectCache; use Wikimedia\Assert\Assert; use Wikimedia\Rdbms\Database; use Wikimedia\Rdbms\IDatabase; +use Wikimedia\Rdbms\ILoadBalancer; use Wikimedia\Rdbms\LoadBalancer; /** @@ -64,7 +65,7 @@ class NameTableStore { private $insertCallback = null; /** - * @param LoadBalancer $dbLoadBalancer A load balancer for acquiring database connections + * @param ILoadBalancer $dbLoadBalancer A load balancer for acquiring database connections * @param WANObjectCache $cache A cache manager for caching data * @param LoggerInterface $logger * @param string $table @@ -77,7 +78,7 @@ class NameTableStore { * This parameter was introduced in 1.32 */ public function __construct( - LoadBalancer $dbLoadBalancer, + ILoadBalancer $dbLoadBalancer, WANObjectCache $cache, LoggerInterface $logger, $table, diff --git a/includes/Storage/RevisionStore.php b/includes/Storage/RevisionStore.php index d090d8be52..0796d620e1 100644 --- a/includes/Storage/RevisionStore.php +++ b/includes/Storage/RevisionStore.php @@ -56,7 +56,7 @@ use Wikimedia\Assert\Assert; use Wikimedia\Rdbms\Database; use Wikimedia\Rdbms\DBConnRef; use Wikimedia\Rdbms\IDatabase; -use Wikimedia\Rdbms\LoadBalancer; +use Wikimedia\Rdbms\ILoadBalancer; /** * Service for looking up page revisions. @@ -88,7 +88,7 @@ class RevisionStore private $contentHandlerUseDB = true; /** - * @var LoadBalancer + * @var ILoadBalancer */ private $loadBalancer; @@ -128,7 +128,7 @@ class RevisionStore /** * @todo $blobStore should be allowed to be any BlobStore! * - * @param LoadBalancer $loadBalancer + * @param ILoadBalancer $loadBalancer * @param SqlBlobStore $blobStore * @param WANObjectCache $cache * @param CommentStore $commentStore @@ -141,7 +141,7 @@ class RevisionStore * @throws MWException if $mcrMigrationStage or $wikiId is invalid. */ public function __construct( - LoadBalancer $loadBalancer, + ILoadBalancer $loadBalancer, SqlBlobStore $blobStore, WANObjectCache $cache, CommentStore $commentStore, @@ -239,7 +239,7 @@ class RevisionStore } /** - * @return LoadBalancer + * @return ILoadBalancer */ private function getDBLoadBalancer() { return $this->loadBalancer; diff --git a/includes/Storage/RevisionStoreFactory.php b/includes/Storage/RevisionStoreFactory.php new file mode 100644 index 0000000000..cfc544451a --- /dev/null +++ b/includes/Storage/RevisionStoreFactory.php @@ -0,0 +1,168 @@ +dbLoadBalancerFactory = $dbLoadBalancerFactory; + $this->blobStoreFactory = $blobStoreFactory; + $this->cache = $cache; + $this->commentStore = $commentStore; + $this->actorMigration = $actorMigration; + $this->mcrMigrationStage = $migrationStage; + $this->loggerProvider = $loggerProvider; + $this->contentHandlerUseDB = $contentHandlerUseDB; + } + /** + + /** + * @since 1.32 + * + * @param bool|string $wikiId false for the current domain / wikid + * + * @return RevisionStore for the given wikiId with all necessary services and a logger + */ + public function getRevisionStore( $wikiId = false ) { + Assert::parameterType( 'string|boolean', $wikiId, '$wikiId' ); + + $store = new RevisionStore( + $this->dbLoadBalancerFactory->getMainLB( $wikiId ), + $this->blobStoreFactory->newSqlBlobStore( $wikiId ), + $this->cache, // Pass local cache instance; Leave cache sharing to RevisionStore. + $this->commentStore, + $this->getContentModelStore( $wikiId ), + $this->getSlotRoleStore( $wikiId ), + $this->mcrMigrationStage, + $this->actorMigration, + $wikiId + ); + + $store->setLogger( $this->loggerProvider->getLogger( 'RevisionStore' ) ); + $store->setContentHandlerUseDB( $this->contentHandlerUseDB ); + + return $store; + } + + /** + * @param string $wikiId + * @return NameTableStore + */ + private function getContentModelStore( $wikiId ) { + // XXX: a dedicated ContentModelStore subclass would avoid hard-coding + // knowledge about the schema here. + return new NameTableStore( + $this->dbLoadBalancerFactory->getMainLB( $wikiId ), + $this->cache, // Pass local cache instance; Leave cache sharing to NameTableStore. + $this->loggerProvider->getLogger( 'NameTableSqlStore' ), + 'content_models', + 'model_id', + 'model_name', + null, + $wikiId + ); + } + + /** + * @param string $wikiId + * @return NameTableStore + */ + private function getSlotRoleStore( $wikiId ) { + // XXX: a dedicated ContentModelStore subclass would avoid hard-coding + // knowledge about the schema here. + return new NameTableStore( + $this->dbLoadBalancerFactory->getMainLB( $wikiId ), + $this->cache, // Pass local cache instance; Leave cache sharing to NameTableStore. + $this->loggerProvider->getLogger( 'NameTableSqlStore' ), + 'slot_roles', + 'role_id', + 'role_name', + 'strtolower', + $wikiId + ); + } + +} diff --git a/tests/phpunit/includes/MediaWikiServicesTest.php b/tests/phpunit/includes/MediaWikiServicesTest.php index ae71d9fac2..413c1a24f4 100644 --- a/tests/phpunit/includes/MediaWikiServicesTest.php +++ b/tests/phpunit/includes/MediaWikiServicesTest.php @@ -16,6 +16,7 @@ use MediaWiki\Storage\NameTableStore; use MediaWiki\Storage\RevisionFactory; use MediaWiki\Storage\RevisionLookup; use MediaWiki\Storage\RevisionStore; +use MediaWiki\Storage\RevisionStoreFactory; use MediaWiki\Storage\SqlBlobStore; /** @@ -348,6 +349,7 @@ class MediaWikiServicesTest extends MediaWikiTestCase { 'BlobStore' => [ 'BlobStore', BlobStore::class ], '_SqlBlobStore' => [ '_SqlBlobStore', SqlBlobStore::class ], 'RevisionStore' => [ 'RevisionStore', RevisionStore::class ], + 'RevisionStoreFactory' => [ 'RevisionStoreFactory', RevisionStoreFactory::class ], 'RevisionLookup' => [ 'RevisionLookup', RevisionLookup::class ], 'RevisionFactory' => [ 'RevisionFactory', RevisionFactory::class ], 'ContentModelStore' => [ 'ContentModelStore', NameTableStore::class ], diff --git a/tests/phpunit/includes/Storage/RevisionStoreFactoryTest.php b/tests/phpunit/includes/Storage/RevisionStoreFactoryTest.php new file mode 100644 index 0000000000..3f8bd4b6ab --- /dev/null +++ b/tests/phpunit/includes/Storage/RevisionStoreFactoryTest.php @@ -0,0 +1,165 @@ +getMockLoadBalancerFactory(), + $this->getMockBlobStoreFactory(), + $this->getHashWANObjectCache(), + $this->getMockCommentStore(), + ActorMigration::newMigration(), + MIGRATION_OLD, + $this->getMockLoggerSpi(), + true + ); + $this->assertTrue( true ); + } + + public function provideWikiIds() { + yield [ true ]; + yield [ false ]; + yield [ 'somewiki' ]; + yield [ 'somewiki', MIGRATION_OLD , false ]; + yield [ 'somewiki', MIGRATION_NEW , true ]; + } + + /** + * @dataProvider provideWikiIds + */ + public function testGetRevisionStore( + $wikiId, + $mcrMigrationStage = MIGRATION_OLD, + $contentHandlerUseDb = true + ) { + $lbFactory = $this->getMockLoadBalancerFactory(); + $blobStoreFactory = $this->getMockBlobStoreFactory(); + $cache = $this->getHashWANObjectCache(); + $commentStore = $this->getMockCommentStore(); + $actorMigration = ActorMigration::newMigration(); + $loggerProvider = $this->getMockLoggerSpi(); + + $factory = new RevisionStoreFactory( + $lbFactory, + $blobStoreFactory, + $cache, + $commentStore, + $actorMigration, + $mcrMigrationStage, + $loggerProvider, + $contentHandlerUseDb + ); + + $store = $factory->getRevisionStore( $wikiId ); + $wrapper = TestingAccessWrapper::newFromObject( $store ); + + // ensure the correct object type is returned + $this->assertInstanceOf( RevisionStore::class, $store ); + + // ensure the RevisionStore is for the given wikiId + $this->assertSame( $wikiId, $wrapper->wikiId ); + + // ensure all other required services are correctly set + $this->assertSame( $cache, $wrapper->cache ); + $this->assertSame( $commentStore, $wrapper->commentStore ); + $this->assertSame( $mcrMigrationStage, $wrapper->mcrMigrationStage ); + $this->assertSame( $actorMigration, $wrapper->actorMigration ); + $this->assertSame( $contentHandlerUseDb, $store->getContentHandlerUseDB() ); + + $this->assertInstanceOf( ILoadBalancer::class, $wrapper->loadBalancer ); + $this->assertInstanceOf( BlobStore::class, $wrapper->blobStore ); + $this->assertInstanceOf( NameTableStore::class, $wrapper->contentModelStore ); + $this->assertInstanceOf( NameTableStore::class, $wrapper->slotRoleStore ); + $this->assertInstanceOf( LoggerInterface::class, $wrapper->logger ); + } + + /** + * @return \PHPUnit_Framework_MockObject_MockObject|ILoadBalancer + */ + private function getMockLoadBalancer() { + return $this->getMockBuilder( ILoadBalancer::class ) + ->disableOriginalConstructor()->getMock(); + } + + /** + * @return \PHPUnit_Framework_MockObject_MockObject|ILBFactory + */ + private function getMockLoadBalancerFactory() { + $mock = $this->getMockBuilder( ILBFactory::class ) + ->disableOriginalConstructor()->getMock(); + + $mock->method( 'getMainLB' ) + ->willReturnCallback( function () { + return $this->getMockLoadBalancer(); + } ); + + return $mock; + } + + /** + * @return \PHPUnit_Framework_MockObject_MockObject|SqlBlobStore + */ + private function getMockSqlBlobStore() { + return $this->getMockBuilder( SqlBlobStore::class ) + ->disableOriginalConstructor()->getMock(); + } + + /** + * @return \PHPUnit_Framework_MockObject_MockObject|BlobStoreFactory + */ + private function getMockBlobStoreFactory() { + $mock = $this->getMockBuilder( BlobStoreFactory::class ) + ->disableOriginalConstructor()->getMock(); + + $mock->method( 'newSqlBlobStore' ) + ->willReturnCallback( function () { + return $this->getMockSqlBlobStore(); + } ); + + return $mock; + } + + /** + * @return \PHPUnit_Framework_MockObject_MockObject|CommentStore + */ + private function getMockCommentStore() { + return $this->getMockBuilder( CommentStore::class ) + ->disableOriginalConstructor()->getMock(); + } + + private function getHashWANObjectCache() { + return new WANObjectCache( [ 'cache' => new \HashBagOStuff() ] ); + } + + /** + * @return \PHPUnit_Framework_MockObject_MockObject|LoggerSpi + */ + private function getMockLoggerSpi() { + $mock = $this->getMock( LoggerSpi::class ); + + $mock->method( 'getLogger' ) + ->willReturn( new NullLogger() ); + + return $mock; + } + +} -- 2.20.1