Introduce RevisionStoreFactory & Tests
authoraddshore <addshorewiki@gmail.com>
Wed, 27 Jun 2018 12:16:35 +0000 (13:16 +0100)
committerDaniel Kinzler <daniel.kinzler@wikimedia.de>
Thu, 2 Aug 2018 18:55:29 +0000 (18:55 +0000)
This is based on I0a8a441b803, which was reverted because it was
incomplete.

Bug: T198701
Change-Id: I3e4a5f1ef687418c06dfc979cfe04da336e876b1

includes/MediaWikiServices.php
includes/ServiceWiring.php
includes/Storage/NameTableStore.php
includes/Storage/RevisionStore.php
includes/Storage/RevisionStoreFactory.php [new file with mode: 0644]
tests/phpunit/includes/MediaWikiServicesTest.php
tests/phpunit/includes/Storage/RevisionStoreFactoryTest.php [new file with mode: 0644]

index 472a453..fd9b472 100644 (file)
@@ -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
index 853b06d..c33dc94 100644 (file)
@@ -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;
        },
 
index 3516ffe..23b2902 100644 (file)
@@ -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,
index d090d8b..0796d62 100644 (file)
@@ -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 (file)
index 0000000..cfc5444
--- /dev/null
@@ -0,0 +1,168 @@
+<?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
+ *
+ * Attribution notice: when this file was created, much of its content was taken
+ * from the Revision.php file as present in release 1.30. Refer to the history
+ * of that file for original authorship.
+ *
+ * @file
+ */
+
+namespace MediaWiki\Storage;
+
+use ActorMigration;
+use CommentStore;
+use MediaWiki\Logger\Spi as LoggerSpi;
+use WANObjectCache;
+use Wikimedia\Assert\Assert;
+use Wikimedia\Rdbms\ILBFactory;
+
+/**
+ * Factory service for RevisionStore instances. This allows RevisionStores to be created for
+ * cross-wiki access.
+ *
+ * @warning Beware compatibility issues with schema migration in the context of cross-wiki access!
+ * This class assumes that all wikis are at compatible migration stages for all relevant schemas.
+ * Relevant schemas are: revision storage (MCR), the revision comment table, and the actor table.
+ * Migration stages are compatible as long as a) there are no wikis in the cluster that only write
+ * the old schema or b) there are no wikis that read only the new schema.
+ *
+ * @since 1.32
+ */
+class RevisionStoreFactory {
+
+       /** @var BlobStoreFactory */
+       private $blobStoreFactory;
+       /** @var ILBFactory */
+       private $dbLoadBalancerFactory;
+       /** @var WANObjectCache */
+       private $cache;
+       /** @var LoggerSpi */
+       private $loggerProvider;
+
+       /** @var CommentStore */
+       private $commentStore;
+       /** @var ActorMigration */
+       private $actorMigration;
+       /** @var int One of the MIGRATION_* constants */
+       private $mcrMigrationStage;
+       /**
+        * @var bool
+        * @see $wgContentHandlerUseDB
+        */
+       private $contentHandlerUseDB;
+
+       /**
+        * @param ILBFactory $dbLoadBalancerFactory
+        * @param BlobStoreFactory $blobStoreFactory
+        * @param WANObjectCache $cache
+        * @param CommentStore $commentStore
+        * @param ActorMigration $actorMigration
+        * @param int $migrationStage
+        * @param LoggerSpi $loggerProvider
+        * @param bool $contentHandlerUseDB see {@link $wgContentHandlerUseDB}
+        */
+       public function __construct(
+               ILBFactory $dbLoadBalancerFactory,
+               BlobStoreFactory $blobStoreFactory,
+               WANObjectCache $cache,
+               CommentStore $commentStore,
+               ActorMigration $actorMigration,
+               $migrationStage,
+               LoggerSpi $loggerProvider,
+               $contentHandlerUseDB
+       ) {
+               Assert::parameterType( 'integer', $migrationStage, '$migrationStage' );
+               $this->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
+               );
+       }
+
+}
index ae71d9f..413c1a2 100644 (file)
@@ -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 (file)
index 0000000..3f8bd4b
--- /dev/null
@@ -0,0 +1,165 @@
+<?php
+
+namespace MediaWiki\Tests\Storage;
+
+use ActorMigration;
+use CommentStore;
+use MediaWiki\Logger\Spi as LoggerSpi;
+use MediaWiki\Storage\BlobStore;
+use MediaWiki\Storage\BlobStoreFactory;
+use MediaWiki\Storage\NameTableStore;
+use MediaWiki\Storage\RevisionStore;
+use MediaWiki\Storage\RevisionStoreFactory;
+use MediaWiki\Storage\SqlBlobStore;
+use MediaWikiTestCase;
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
+use WANObjectCache;
+use Wikimedia\Rdbms\ILBFactory;
+use Wikimedia\Rdbms\ILoadBalancer;
+use Wikimedia\TestingAccessWrapper;
+
+class RevisionStoreFactoryTest extends MediaWikiTestCase {
+
+       public function testValidConstruction_doesntCauseErrors() {
+               new RevisionStoreFactory(
+                       $this->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;
+       }
+
+}