Merge "Re-apply: Factors out permissions check from User into PermissionManager service"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Fri, 28 Jun 2019 23:44:38 +0000 (23:44 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Fri, 28 Jun 2019 23:44:38 +0000 (23:44 +0000)
51 files changed:
RELEASE-NOTES-1.34
autoload.php
includes/MediaWikiServices.php
includes/Revision/MutableRevisionRecord.php
includes/Revision/RevisionArchiveRecord.php
includes/Revision/RevisionRecord.php
includes/Revision/RevisionRenderer.php
includes/Revision/RevisionStore.php
includes/Revision/RevisionStoreCacheRecord.php
includes/Revision/RevisionStoreFactory.php
includes/Revision/RevisionStoreRecord.php
includes/ServiceWiring.php
includes/Storage/BlobStoreFactory.php
includes/Storage/SqlBlobStore.php
includes/db/DatabaseOracle.php
includes/externalstore/ExternalStore.php
includes/externalstore/ExternalStoreAccess.php [new file with mode: 0644]
includes/externalstore/ExternalStoreDB.php
includes/externalstore/ExternalStoreException.php [new file with mode: 0644]
includes/externalstore/ExternalStoreFactory.php
includes/externalstore/ExternalStoreMedium.php
includes/externalstore/ExternalStoreMemory.php [new file with mode: 0644]
includes/externalstore/ExternalStoreMwstore.php
includes/historyblob/HistoryBlobStub.php
includes/libs/filebackend/SwiftFileBackend.php
includes/libs/rdbms/connectionmanager/ConnectionManager.php
includes/libs/rdbms/connectionmanager/SessionConsistentConnectionManager.php
includes/libs/rdbms/database/Database.php
includes/libs/rdbms/database/DatabaseMssql.php
includes/libs/rdbms/database/DatabasePostgres.php
includes/libs/rdbms/database/DatabaseSqlite.php
includes/libs/rdbms/database/resultwrapper/FakeResultWrapper.php
includes/libs/rdbms/lbfactory/ILBFactory.php
includes/libs/rdbms/lbfactory/LBFactoryMulti.php
includes/libs/rdbms/lbfactory/LBFactorySimple.php
includes/libs/rdbms/loadbalancer/ILoadBalancer.php
includes/libs/rdbms/loadbalancer/LoadBalancer.php
maintenance/storage/checkStorage.php
maintenance/storage/compressOld.php
maintenance/storage/moveToExternal.php
maintenance/storage/recompressTracked.php
resources/src/mediawiki.widgets.datetime/DateTimeInputWidget.js
tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php
tests/phpunit/includes/Revision/RevisionStoreFactoryTest.php
tests/phpunit/includes/Revision/RevisionStoreTest.php
tests/phpunit/includes/RevisionTest.php
tests/phpunit/includes/Storage/SqlBlobStoreTest.php
tests/phpunit/includes/externalstore/ExternalStoreAccessTest.php [new file with mode: 0644]
tests/phpunit/includes/externalstore/ExternalStoreFactoryTest.php
tests/phpunit/includes/externalstore/ExternalStoreTest.php
tests/phpunit/includes/parser/ParserOutputTest.php

index fdc9e05..acd82d6 100644 (file)
@@ -327,6 +327,8 @@ because of Phabricator reports.
   engines.
 * Skin::escapeSearchLink() is deprecated. Use Skin::getSearchLink() or the skin
   template option 'searchaction' instead.
+* LoadBalancer::haveIndex() and LoadBalancer::isNonZeroLoad() have
+  been deprecated.
 
 === Other changes in 1.34 ===
 * …
index b01827a..6457747 100644 (file)
@@ -481,10 +481,13 @@ $wgAutoloadLocalClasses = [
        'ExtensionProcessor' => __DIR__ . '/includes/registration/ExtensionProcessor.php',
        'ExtensionRegistry' => __DIR__ . '/includes/registration/ExtensionRegistry.php',
        'ExternalStore' => __DIR__ . '/includes/externalstore/ExternalStore.php',
+       'ExternalStoreAccess' => __DIR__ . '/includes/externalstore/ExternalStoreAccess.php',
        'ExternalStoreDB' => __DIR__ . '/includes/externalstore/ExternalStoreDB.php',
+       'ExternalStoreException' => __DIR__ . '/includes/externalstore/ExternalStoreException.php',
        'ExternalStoreFactory' => __DIR__ . '/includes/externalstore/ExternalStoreFactory.php',
        'ExternalStoreHttp' => __DIR__ . '/includes/externalstore/ExternalStoreHttp.php',
        'ExternalStoreMedium' => __DIR__ . '/includes/externalstore/ExternalStoreMedium.php',
+       'ExternalStoreMemory' => __DIR__ . '/includes/externalstore/ExternalStoreMemory.php',
        'ExternalStoreMwstore' => __DIR__ . '/includes/externalstore/ExternalStoreMwstore.php',
        'ExternalUserNames' => __DIR__ . '/includes/user/ExternalUserNames.php',
        'FSFile' => __DIR__ . '/includes/libs/filebackend/fsfile/FSFile.php',
index 689477b..a37e32e 100644 (file)
@@ -571,6 +571,14 @@ class MediaWikiServices extends ServiceContainer {
                return $this->getService( 'EventRelayerGroup' );
        }
 
+       /**
+        * @since 1.34
+        * @return \ExternalStoreAccess
+        */
+       public function getExternalStoreAccess() {
+               return $this->getService( 'ExternalStoreAccess' );
+       }
+
        /**
         * @since 1.31
         * @return \ExternalStoreFactory
index f287c05..e9136cb 100644 (file)
@@ -70,15 +70,14 @@ class MutableRevisionRecord extends RevisionRecord {
         * in RevisionStore instead.
         *
         * @param Title $title The title of the page this Revision is associated with.
-        * @param bool|string $wikiId the wiki ID of the site this Revision belongs to,
-        *        or false for the local site.
+        * @param bool|string $dbDomain DB domain of the relevant wiki or false for the current one.
         *
         * @throws MWException
         */
-       function __construct( Title $title, $wikiId = false ) {
+       function __construct( Title $title, $dbDomain = false ) {
                $slots = new MutableRevisionSlots();
 
-               parent::__construct( $title, $slots, $wikiId );
+               parent::__construct( $title, $slots, $dbDomain );
 
                $this->mSlots = $slots; // redundant, but nice for static analysis
        }
index 67dc9b2..6e8db7f 100644 (file)
@@ -54,8 +54,7 @@ class RevisionArchiveRecord extends RevisionRecord {
         * @param object $row An archive table row. Use RevisionStore::getArchiveQueryInfo() to build
         *        a query that yields the required fields.
         * @param RevisionSlots $slots The slots of this revision.
-        * @param bool|string $wikiId the wiki ID of the site this Revision belongs to,
-        *        or false for the local site.
+        * @param bool|string $dbDomain DB domain of the relevant wiki or false for the current one.
         */
        function __construct(
                Title $title,
@@ -63,9 +62,9 @@ class RevisionArchiveRecord extends RevisionRecord {
                CommentStoreComment $comment,
                $row,
                RevisionSlots $slots,
-               $wikiId = false
+               $dbDomain = false
        ) {
-               parent::__construct( $title, $slots, $wikiId );
+               parent::__construct( $title, $slots, $dbDomain );
                Assert::parameterType( 'object', $row, '$row' );
 
                $timestamp = wfTimestamp( TS_MW, $row->ar_timestamp );
index 7bd127e..0dcc35c 100644 (file)
@@ -94,17 +94,16 @@ abstract class RevisionRecord {
         *
         * @param Title $title The title of the page this Revision is associated with.
         * @param RevisionSlots $slots The slots of this revision.
-        * @param bool|string $wikiId the wiki ID of the site this Revision belongs to,
-        *        or false for the local site.
+        * @param bool|string $dbDomain DB domain of the relevant wiki or false for the current one.
         *
         * @throws MWException
         */
-       function __construct( Title $title, RevisionSlots $slots, $wikiId = false ) {
-               Assert::parameterType( 'string|boolean', $wikiId, '$wikiId' );
+       function __construct( Title $title, RevisionSlots $slots, $dbDomain = false ) {
+               Assert::parameterType( 'string|boolean', $dbDomain, '$dbDomain' );
 
                $this->mTitle = $title;
                $this->mSlots = $slots;
-               $this->mWiki = $wikiId;
+               $this->mWiki = $dbDomain;
 
                // XXX: this is a sensible default, but we may not have a Title object here in the future.
                $this->mPageId = $title->getArticleID();
index a63e4f1..99150c1 100644 (file)
@@ -54,22 +54,21 @@ class RevisionRenderer {
        private $roleRegistery;
 
        /** @var string|bool */
-       private $wikiId;
+       private $dbDomain;
 
        /**
         * @param ILoadBalancer $loadBalancer
         * @param SlotRoleRegistry $roleRegistry
-        * @param bool|string $wikiId
+        * @param bool|string $dbDomain DB domain of the relevant wiki or false for the current one
         */
        public function __construct(
                ILoadBalancer $loadBalancer,
                SlotRoleRegistry $roleRegistry,
-               $wikiId = false
+               $dbDomain = false
        ) {
                $this->loadBalancer = $loadBalancer;
                $this->roleRegistery = $roleRegistry;
-               $this->wikiId = $wikiId;
-
+               $this->dbDomain = $dbDomain;
                $this->saveParseLogger = new NullLogger();
        }
 
@@ -105,7 +104,7 @@ class RevisionRenderer {
                User $forUser = null,
                array $hints = []
        ) {
-               if ( $rev->getWikiId() !== $this->wikiId ) {
+               if ( $rev->getWikiId() !== $this->dbDomain ) {
                        throw new InvalidArgumentException( 'Mismatching wiki ID ' . $rev->getWikiId() );
                }
 
@@ -169,7 +168,7 @@ class RevisionRenderer {
                $flags = defined( 'MW_PHPUNIT_TEST' ) || $dbIndex === DB_REPLICA
                        ? 0 : ILoadBalancer::CONN_TRX_AUTOCOMMIT;
 
-               $db = $this->loadBalancer->getConnectionRef( $dbIndex, [], $this->wikiId, $flags );
+               $db = $this->loadBalancer->getConnectionRef( $dbIndex, [], $this->dbDomain, $flags );
 
                return 1 + (int)$db->selectField(
                        'revision',
@@ -216,7 +215,7 @@ class RevisionRenderer {
                        $slotOutput[$role] = $out;
 
                        // XXX: should the SlotRoleHandler be able to intervene here?
-                       $combinedOutput->mergeInternalMetaDataFrom( $out, $role );
+                       $combinedOutput->mergeInternalMetaDataFrom( $out );
                        $combinedOutput->mergeTrackingMetaDataFrom( $out );
                }
 
index 56867eb..f269afe 100644 (file)
@@ -87,7 +87,7 @@ class RevisionStore
        /**
         * @var bool|string
         */
-       private $wikiId;
+       private $dbDomain;
 
        /**
         * @var boolean
@@ -142,7 +142,7 @@ class RevisionStore
         * @param ILoadBalancer $loadBalancer
         * @param SqlBlobStore $blobStore
         * @param WANObjectCache $cache A cache for caching revision rows. This can be the local
-        *        wiki's default instance even if $wikiId refers to a different wiki, since
+        *        wiki's default instance even if $dbDomain refers to a different wiki, since
         *        makeGlobalKey() is used to constructed a key that allows cached revision rows from
         *        the same database to be re-used between wikis. For example, enwiki and frwiki will
         *        use the same cache keys for revision rows from the wikidatawiki database, regardless
@@ -153,8 +153,7 @@ class RevisionStore
         * @param SlotRoleRegistry $slotRoleRegistry
         * @param int $mcrMigrationStage An appropriate combination of SCHEMA_COMPAT_XXX flags
         * @param ActorMigration $actorMigration
-        * @param bool|string $wikiId
-        *
+        * @param bool|string $dbDomain DB domain of the relevant wiki or false for the current one
         */
        public function __construct(
                ILoadBalancer $loadBalancer,
@@ -166,9 +165,9 @@ class RevisionStore
                SlotRoleRegistry $slotRoleRegistry,
                $mcrMigrationStage,
                ActorMigration $actorMigration,
-               $wikiId = false
+               $dbDomain = false
        ) {
-               Assert::parameterType( 'string|boolean', $wikiId, '$wikiId' );
+               Assert::parameterType( 'string|boolean', $dbDomain, '$dbDomain' );
                Assert::parameterType( 'integer', $mcrMigrationStage, '$mcrMigrationStage' );
                Assert::parameter(
                        ( $mcrMigrationStage & SCHEMA_COMPAT_READ_BOTH ) !== SCHEMA_COMPAT_READ_BOTH,
@@ -207,7 +206,7 @@ class RevisionStore
                $this->slotRoleRegistry = $slotRoleRegistry;
                $this->mcrMigrationStage = $mcrMigrationStage;
                $this->actorMigration = $actorMigration;
-               $this->wikiId = $wikiId;
+               $this->dbDomain = $dbDomain;
                $this->logger = new NullLogger();
        }
 
@@ -227,7 +226,7 @@ class RevisionStore
         * @throws RevisionAccessException
         */
        private function assertCrossWikiContentLoadingIsSafe() {
-               if ( $this->wikiId !== false && $this->hasMcrSchemaFlags( SCHEMA_COMPAT_READ_OLD ) ) {
+               if ( $this->dbDomain !== false && $this->hasMcrSchemaFlags( SCHEMA_COMPAT_READ_OLD ) ) {
                        throw new RevisionAccessException(
                                "Cross-wiki content loading is not supported by the pre-MCR schema"
                        );
@@ -285,7 +284,7 @@ class RevisionStore
         */
        private function getDBConnection( $mode, $groups = [] ) {
                $lb = $this->getDBLoadBalancer();
-               return $lb->getConnection( $mode, $groups, $this->wikiId );
+               return $lb->getConnection( $mode, $groups, $this->dbDomain );
        }
 
        /**
@@ -313,7 +312,7 @@ class RevisionStore
         */
        private function getDBConnectionRef( $mode ) {
                $lb = $this->getDBLoadBalancer();
-               return $lb->getConnectionRef( $mode, [], $this->wikiId );
+               return $lb->getConnectionRef( $mode, [], $this->dbDomain );
        }
 
        /**
@@ -341,7 +340,7 @@ class RevisionStore
                        $queryFlags = self::READ_NORMAL;
                }
 
-               $canUseTitleNewFromId = ( $pageId !== null && $pageId > 0 && $this->wikiId === false );
+               $canUseTitleNewFromId = ( $pageId !== null && $pageId > 0 && $this->dbDomain === false );
                list( $dbMode, $dbOptions ) = DBAccessObjectUtils::getDBOptions( $queryFlags );
                $titleFlags = ( $dbMode == DB_MASTER ? Title::GAID_FOR_UPDATE : 0 );
 
@@ -631,7 +630,7 @@ class RevisionStore
                        $comment,
                        (object)$revisionRow,
                        new RevisionSlots( $newSlots ),
-                       $this->wikiId
+                       $this->dbDomain
                );
 
                return $rev;
@@ -813,9 +812,11 @@ class RevisionStore
                                                throw new MWException( 'Failed to get database lock for T202032' );
                                        }
                                        $fname = __METHOD__;
-                                       $dbw->onTransactionResolution( function ( $trigger, $dbw ) use ( $fname ) {
-                                               $dbw->unlock( 'fix-for-T202032', $fname );
-                                       } );
+                                       $dbw->onTransactionResolution(
+                                               function ( $trigger, IDatabase $dbw ) use ( $fname ) {
+                                                       $dbw->unlock( 'fix-for-T202032', $fname );
+                                               }
+                                       );
 
                                        $dbw->delete( 'revision', [ 'rev_id' => $revisionRow['rev_id'] ], __METHOD__ );
 
@@ -1782,7 +1783,7 @@ class RevisionStore
                                $row->ar_user ?? null,
                                $row->ar_user_text ?? null,
                                $row->ar_actor ?? null,
-                               $this->wikiId
+                               $this->dbDomain
                        );
                } catch ( InvalidArgumentException $ex ) {
                        wfWarn( __METHOD__ . ': ' . $title->getPrefixedDBkey() . ': ' . $ex->getMessage() );
@@ -1795,7 +1796,7 @@ class RevisionStore
 
                $slots = $this->newRevisionSlots( $row->ar_rev_id, $row, null, $queryFlags, $title );
 
-               return new RevisionArchiveRecord( $title, $user, $comment, $row, $slots, $this->wikiId );
+               return new RevisionArchiveRecord( $title, $user, $comment, $row, $slots, $this->dbDomain );
        }
 
        /**
@@ -1863,7 +1864,7 @@ class RevisionStore
                                $row->rev_user ?? null,
                                $row->rev_user_text ?? null,
                                $row->rev_actor ?? null,
-                               $this->wikiId
+                               $this->dbDomain
                        );
                } catch ( InvalidArgumentException $ex ) {
                        wfWarn( __METHOD__ . ': ' . $title->getPrefixedDBkey() . ': ' . $ex->getMessage() );
@@ -1886,11 +1887,11 @@ class RevisionStore
                                                [ 'rev_id' => intval( $revId ) ]
                                        );
                                },
-                               $title, $user, $comment, $row, $slots, $this->wikiId
+                               $title, $user, $comment, $row, $slots, $this->dbDomain
                        );
                } else {
                        $rev = new RevisionStoreRecord(
-                               $title, $user, $comment, $row, $slots, $this->wikiId );
+                               $title, $user, $comment, $row, $slots, $this->dbDomain );
                }
                return $rev;
        }
@@ -1975,7 +1976,7 @@ class RevisionStore
                        }
                }
 
-               $revision = new MutableRevisionRecord( $title, $this->wikiId );
+               $revision = new MutableRevisionRecord( $title, $this->dbDomain );
                $this->initializeMutableRevisionFromArray( $revision, $fields );
 
                if ( isset( $fields['content'] ) && is_array( $fields['content'] ) ) {
@@ -2006,7 +2007,7 @@ class RevisionStore
                // remote wiki with unsuppressed ids, due to issues described in T222212.
                if ( isset( $fields['user'] ) &&
                        ( $fields['user'] instanceof UserIdentity ) &&
-                       ( $this->wikiId === false ||
+                       ( $this->dbDomain === false ||
                                ( !$fields['user']->getId() && !$fields['user']->getActorId() ) )
                ) {
                        $user = $fields['user'];
@@ -2016,7 +2017,7 @@ class RevisionStore
                                        $fields['user'] ?? null,
                                        $fields['user_text'] ?? null,
                                        $fields['actor'] ?? null,
-                                       $this->wikiId
+                                       $this->dbDomain
                                );
                        } catch ( InvalidArgumentException $ex ) {
                                $user = null;
@@ -2247,7 +2248,7 @@ class RevisionStore
         * @throws MWException
         */
        private function checkDatabaseWikiId( IDatabase $db ) {
-               $storeWiki = $this->wikiId;
+               $storeWiki = $this->dbDomain;
                $dbWiki = $db->getDomainID();
 
                if ( $dbWiki === $storeWiki ) {
index ef5f10e..0420d34 100644 (file)
@@ -53,8 +53,7 @@ class RevisionStoreCacheRecord extends RevisionStoreRecord {
         * @param object $row A row from the revision table. Use RevisionStore::getQueryInfo() to build
         *        a query that yields the required fields.
         * @param RevisionSlots $slots The slots of this revision.
-        * @param bool|string $wikiId the wiki ID of the site this Revision belongs to,
-        *        or false for the local site.
+        * @param bool|string $dbDomain DB domain of the relevant wiki or false for the current one.
         */
        function __construct(
                $callback,
@@ -63,9 +62,9 @@ class RevisionStoreCacheRecord extends RevisionStoreRecord {
                CommentStoreComment $comment,
                $row,
                RevisionSlots $slots,
-               $wikiId = false
+               $dbDomain = false
        ) {
-               parent::__construct( $title, $user, $comment, $row, $slots, $wikiId );
+               parent::__construct( $title, $user, $comment, $row, $slots, $dbDomain );
                $this->mCallback = $callback;
        }
 
index 6b3117f..0475557 100644 (file)
@@ -116,24 +116,24 @@ class RevisionStoreFactory {
        /**
         * @since 1.32
         *
-        * @param bool|string $wikiId false for the current domain / wikid
+        * @param bool|string $dbDomain DB domain of the relevant wiki or false for the current one
         *
         * @return RevisionStore for the given wikiId with all necessary services and a logger
         */
-       public function getRevisionStore( $wikiId = false ) {
-               Assert::parameterType( 'string|boolean', $wikiId, '$wikiId' );
+       public function getRevisionStore( $dbDomain = false ) {
+               Assert::parameterType( 'string|boolean', $dbDomain, '$dbDomain' );
 
                $store = new RevisionStore(
-                       $this->dbLoadBalancerFactory->getMainLB( $wikiId ),
-                       $this->blobStoreFactory->newSqlBlobStore( $wikiId ),
+                       $this->dbLoadBalancerFactory->getMainLB( $dbDomain ),
+                       $this->blobStoreFactory->newSqlBlobStore( $dbDomain ),
                        $this->cache, // Pass local cache instance; Leave cache sharing to RevisionStore.
                        $this->commentStore,
-                       $this->nameTables->getContentModels( $wikiId ),
-                       $this->nameTables->getSlotRoles( $wikiId ),
+                       $this->nameTables->getContentModels( $dbDomain ),
+                       $this->nameTables->getSlotRoles( $dbDomain ),
                        $this->slotRoleRegistry,
                        $this->mcrMigrationStage,
                        $this->actorMigration,
-                       $wikiId
+                       $dbDomain
                );
 
                $store->setLogger( $this->loggerProvider->getLogger( 'RevisionStore' ) );
index 955cc82..469e494 100644 (file)
@@ -51,8 +51,7 @@ class RevisionStoreRecord extends RevisionRecord {
         * @param object $row A row from the revision table. Use RevisionStore::getQueryInfo() to build
         *        a query that yields the required fields.
         * @param RevisionSlots $slots The slots of this revision.
-        * @param bool|string $wikiId the wiki ID of the site this Revision belongs to,
-        *        or false for the local site.
+        * @param bool|string $dbDomain DB domain of the relevant wiki or false for the current one.
         */
        function __construct(
                Title $title,
@@ -60,9 +59,9 @@ class RevisionStoreRecord extends RevisionRecord {
                CommentStoreComment $comment,
                $row,
                RevisionSlots $slots,
-               $wikiId = false
+               $dbDomain = false
        ) {
-               parent::__construct( $title, $slots, $wikiId );
+               parent::__construct( $title, $slots, $dbDomain );
                Assert::parameterType( 'object', $row, '$row' );
 
                $this->mId = intval( $row->rev_id );
index 041bb51..96baf14 100644 (file)
@@ -82,6 +82,7 @@ return [
        'BlobStoreFactory' => function ( MediaWikiServices $services ) : BlobStoreFactory {
                return new BlobStoreFactory(
                        $services->getDBLoadBalancerFactory(),
+                       $services->getExternalStoreAccess(),
                        $services->getMainWANObjectCache(),
                        new ServiceOptions( BlobStoreFactory::$constructorOptions,
                                $services->getMainConfig() ),
@@ -201,11 +202,22 @@ return [
                return new EventRelayerGroup( $services->getMainConfig()->get( 'EventRelayerConfig' ) );
        },
 
+       'ExternalStoreAccess' => function ( MediaWikiServices $services ) : ExternalStoreAccess {
+               return new ExternalStoreAccess(
+                       $services->getExternalStoreFactory(),
+                       LoggerFactory::getInstance( 'ExternalStore' )
+               );
+       },
+
        'ExternalStoreFactory' => function ( MediaWikiServices $services ) : ExternalStoreFactory {
                $config = $services->getMainConfig();
+               $writeStores = $config->get( 'DefaultExternalStore' );
 
                return new ExternalStoreFactory(
-                       $config->get( 'ExternalStores' )
+                       $config->get( 'ExternalStores' ),
+                       ( $writeStores !== false ) ? (array)$writeStores : [],
+                       $services->getDBLoadBalancer()->getLocalDomainID(),
+                       LoggerFactory::getInstance( 'ExternalStore' )
                );
        },
 
index 8262446..b59c68d 100644 (file)
@@ -24,6 +24,7 @@ use Language;
 use MediaWiki\Config\ServiceOptions;
 use WANObjectCache;
 use Wikimedia\Rdbms\ILBFactory;
+use ExternalStoreAccess;
 
 /**
  * Service for instantiating BlobStores
@@ -39,6 +40,11 @@ class BlobStoreFactory {
         */
        private $lbFactory;
 
+       /**
+        * @var ExternalStoreAccess
+        */
+       private $extStoreAccess;
+
        /**
         * @var WANObjectCache
         */
@@ -69,6 +75,7 @@ class BlobStoreFactory {
 
        public function __construct(
                ILBFactory $lbFactory,
+               ExternalStoreAccess $extStoreAccess,
                WANObjectCache $cache,
                ServiceOptions $options,
                Language $contLang
@@ -76,6 +83,7 @@ class BlobStoreFactory {
                $options->assertRequiredOptions( self::$constructorOptions );
 
                $this->lbFactory = $lbFactory;
+               $this->extStoreAccess = $extStoreAccess;
                $this->cache = $cache;
                $this->options = $options;
                $this->contLang = $contLang;
@@ -103,6 +111,7 @@ class BlobStoreFactory {
                $lb = $this->lbFactory->getMainLB( $dbDomain );
                $store = new SqlBlobStore(
                        $lb,
+                       $this->extStoreAccess,
                        $this->cache,
                        $dbDomain
                );
index 7fe5643..5260754 100644 (file)
 namespace MediaWiki\Storage;
 
 use DBAccessObjectUtils;
-use ExternalStore;
 use IDBAccessObject;
 use IExpiringStore;
 use InvalidArgumentException;
 use Language;
 use MWException;
 use WANObjectCache;
+use ExternalStoreAccess;
 use Wikimedia\Assert\Assert;
 use Wikimedia\Rdbms\IDatabase;
 use Wikimedia\Rdbms\ILoadBalancer;
@@ -56,13 +56,18 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
         */
        private $dbLoadBalancer;
 
+       /**
+        * @var ExternalStoreAccess
+        */
+       private $extStoreAccess;
+
        /**
         * @var WANObjectCache
         */
        private $cache;
 
        /**
-        * @var bool|string Wiki ID
+        * @var string|bool DB domain ID of a wiki or false for the local one
         */
        private $dbDomain;
 
@@ -93,6 +98,7 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
 
        /**
         * @param ILoadBalancer $dbLoadBalancer A load balancer for acquiring database connections
+        * @param ExternalStoreAccess $extStoreAccess Access layer for external storage
         * @param WANObjectCache $cache A cache manager for caching blobs. This can be the local
         *        wiki's default instance even if $dbDomain refers to a different wiki, since
         *        makeGlobalKey() is used to constructed a key that allows cached blobs from the
@@ -103,10 +109,12 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
         */
        public function __construct(
                ILoadBalancer $dbLoadBalancer,
+               ExternalStoreAccess $extStoreAccess,
                WANObjectCache $cache,
                $dbDomain = false
        ) {
                $this->dbLoadBalancer = $dbLoadBalancer;
+               $this->extStoreAccess = $extStoreAccess;
                $this->cache = $cache;
                $this->dbDomain = $dbDomain;
        }
@@ -219,7 +227,10 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
                        # Write to external storage if required
                        if ( $this->useExternalStore ) {
                                // Store and get the URL
-                               $data = ExternalStore::insertToDefault( $data, [ 'wiki' => $this->dbDomain ] );
+                               $data = $this->extStoreAccess->insert( $data, [ 'domain' => $this->dbDomain ] );
+                               if ( !$data ) {
+                                       throw new BlobAccessException( "Failed to store text to external storage" );
+                               }
                                if ( $flags ) {
                                        $flags .= ',';
                                }
@@ -412,14 +423,15 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
                                        $this->getCacheTTL(),
                                        function () use ( $url, $flags ) {
                                                // Ignore $setOpts; blobs are immutable and negatives are not cached
-                                               $blob = ExternalStore::fetchFromURL( $url, [ 'wiki' => $this->dbDomain ] );
+                                               $blob = $this->extStoreAccess
+                                                       ->fetchFromURL( $url, [ 'domain' => $this->dbDomain ] );
 
                                                return $blob === false ? false : $this->decompressData( $blob, $flags );
                                        },
                                        [ 'pcGroup' => self::TEXT_CACHE_GROUP, 'pcTTL' => WANObjectCache::TTL_PROC_LONG ]
                                );
                        } else {
-                               $blob = ExternalStore::fetchFromURL( $url, [ 'wiki' => $this->dbDomain ] );
+                               $blob = $this->extStoreAccess->fetchFromURL( $url, [ 'domain' => $this->dbDomain ] );
                                return $blob === false ? false : $this->decompressData( $blob, $flags );
                        }
                } else {
@@ -623,7 +635,7 @@ class SqlBlobStore implements IDBAccessObject, BlobStore {
        }
 
        public function isReadOnly() {
-               if ( $this->useExternalStore && ExternalStore::defaultStoresAreReadOnly() ) {
+               if ( $this->useExternalStore && $this->extStoreAccess->isReadOnly() ) {
                        return true;
                }
 
index 4af62a0..c716e4d 100644 (file)
@@ -178,7 +178,7 @@ class DatabaseOracle extends Database {
        }
 
        function execFlags() {
-               return $this->trxLevel ? OCI_NO_AUTO_COMMIT : OCI_COMMIT_ON_SUCCESS;
+               return $this->trxLevel() ? OCI_NO_AUTO_COMMIT : OCI_COMMIT_ON_SUCCESS;
        }
 
        /**
@@ -548,7 +548,7 @@ class DatabaseOracle extends Database {
                        }
                }
 
-               if ( !$this->trxLevel ) {
+               if ( !$this->trxLevel() ) {
                        oci_commit( $this->conn );
                }
 
@@ -942,26 +942,24 @@ class DatabaseOracle extends Database {
        }
 
        protected function doBegin( $fname = __METHOD__ ) {
-               $this->trxLevel = 1;
-               $this->doQuery( 'SET CONSTRAINTS ALL DEFERRED' );
+               $this->query( 'SET CONSTRAINTS ALL DEFERRED' );
        }
 
        protected function doCommit( $fname = __METHOD__ ) {
-               if ( $this->trxLevel ) {
+               if ( $this->trxLevel() ) {
                        $ret = oci_commit( $this->conn );
                        if ( !$ret ) {
                                throw new DBUnexpectedError( $this, $this->lastError() );
                        }
-                       $this->trxLevel = 0;
-                       $this->doQuery( 'SET CONSTRAINTS ALL IMMEDIATE' );
+                       $this->query( 'SET CONSTRAINTS ALL IMMEDIATE' );
                }
        }
 
        protected function doRollback( $fname = __METHOD__ ) {
-               if ( $this->trxLevel ) {
+               if ( $this->trxLevel() ) {
                        oci_rollback( $this->conn );
-                       $this->trxLevel = 0;
-                       $this->doQuery( 'SET CONSTRAINTS ALL IMMEDIATE' );
+                       $ignoreErrors = true;
+                       $this->query( 'SET CONSTRAINTS ALL IMMEDIATE', $fname, $ignoreErrors );
                }
        }
 
@@ -1338,7 +1336,7 @@ class DatabaseOracle extends Database {
                        }
                }
 
-               if ( !$this->trxLevel ) {
+               if ( !$this->trxLevel() ) {
                        oci_commit( $this->conn );
                }
 
index 76f20f0..7c90b35 100644 (file)
@@ -44,6 +44,7 @@ use MediaWiki\MediaWikiServices;
  * as the possibility to have any storage format (i.e. for archives).
  *
  * @ingroup ExternalStorage
+ * @deprecated 1.34 Use ExternalStoreFactory directly instead
  */
 class ExternalStore {
        /**
@@ -52,11 +53,16 @@ class ExternalStore {
         * @param string $proto Type of external storage, should be a value in $wgExternalStores
         * @param array $params Associative array of ExternalStoreMedium parameters
         * @return ExternalStoreMedium|bool The store class or false on error
+        * @deprecated 1.34
         */
        public static function getStoreObject( $proto, array $params = [] ) {
-               return MediaWikiServices::getInstance()
-                       ->getExternalStoreFactory()
-                       ->getStoreObject( $proto, $params );
+               try {
+                       return MediaWikiServices::getInstance()
+                               ->getExternalStoreFactory()
+                               ->getStore( $proto, $params );
+               } catch ( ExternalStoreException $e ) {
+                       return false;
+               }
        }
 
        /**
@@ -66,59 +72,16 @@ class ExternalStore {
         * @param array $params Associative array of ExternalStoreMedium parameters
         * @return string|bool The text stored or false on error
         * @throws MWException
+        * @deprecated 1.34
         */
        public static function fetchFromURL( $url, array $params = [] ) {
-               $parts = explode( '://', $url, 2 );
-               if ( count( $parts ) != 2 ) {
-                       return false; // invalid URL
-               }
-
-               list( $proto, $path ) = $parts;
-               if ( $path == '' ) { // bad URL
-                       return false;
-               }
-
-               $store = self::getStoreObject( $proto, $params );
-               if ( $store === false ) {
+               try {
+                       return MediaWikiServices::getInstance()
+                               ->getExternalStoreAccess()
+                               ->fetchFromURL( $url, $params );
+               } catch ( ExternalStoreException $e ) {
                        return false;
                }
-
-               return $store->fetchFromURL( $url );
-       }
-
-       /**
-        * Fetch data from multiple URLs with a minimum of round trips
-        *
-        * @param array $urls The URLs of the text to get
-        * @return array Map from url to its data.  Data is either string when found
-        *     or false on failure.
-        * @throws MWException
-        */
-       public static function batchFetchFromURLs( array $urls ) {
-               $batches = [];
-               foreach ( $urls as $url ) {
-                       $scheme = parse_url( $url, PHP_URL_SCHEME );
-                       if ( $scheme ) {
-                               $batches[$scheme][] = $url;
-                       }
-               }
-               $retval = [];
-               foreach ( $batches as $proto => $batchedUrls ) {
-                       $store = self::getStoreObject( $proto );
-                       if ( $store === false ) {
-                               continue;
-                       }
-                       $retval += $store->batchFetchFromURLs( $batchedUrls );
-               }
-               // invalid, not found, db dead, etc.
-               $missing = array_diff( $urls, array_keys( $retval ) );
-               if ( $missing ) {
-                       foreach ( $missing as $url ) {
-                               $retval[$url] = false;
-                       }
-               }
-
-               return $retval;
        }
 
        /**
@@ -131,24 +94,30 @@ class ExternalStore {
         * @param array $params Associative array of ExternalStoreMedium parameters
         * @return string|bool The URL of the stored data item, or false on error
         * @throws MWException
+        * @deprecated 1.34
         */
        public static function insert( $url, $data, array $params = [] ) {
-               $parts = explode( '://', $url, 2 );
-               if ( count( $parts ) != 2 ) {
-                       return false; // invalid URL
-               }
+               try {
+                       $esFactory = MediaWikiServices::getInstance()->getExternalStoreFactory();
+                       $location = $esFactory->getStoreLocationFromUrl( $url );
 
-               list( $proto, $path ) = $parts;
-               if ( $path == '' ) { // bad URL
+                       return $esFactory->getStoreForUrl( $url, $params )->store( $location, $data );
+               } catch ( ExternalStoreException $e ) {
                        return false;
                }
+       }
 
-               $store = self::getStoreObject( $proto, $params );
-               if ( $store === false ) {
-                       return false;
-               } else {
-                       return $store->store( $path, $data );
-               }
+       /**
+        * Fetch data from multiple URLs with a minimum of round trips
+        *
+        * @param array $urls The URLs of the text to get
+        * @return array Map from url to its data.  Data is either string when found
+        *     or false on failure.
+        * @throws MWException
+        * @deprecated 1.34
+        */
+       public static function batchFetchFromURLs( array $urls ) {
+               return MediaWikiServices::getInstance()->getExternalStoreAccess()->fetchFromURLs( $urls );
        }
 
        /**
@@ -161,11 +130,10 @@ class ExternalStore {
         * @param array $params Map of ExternalStoreMedium::__construct context parameters
         * @return string The URL of the stored data item
         * @throws MWException
+        * @deprecated 1.34
         */
        public static function insertToDefault( $data, array $params = [] ) {
-               global $wgDefaultExternalStore;
-
-               return self::insertWithFallback( (array)$wgDefaultExternalStore, $data, $params );
+               return MediaWikiServices::getInstance()->getExternalStoreAccess()->insert( $data, $params );
        }
 
        /**
@@ -179,67 +147,12 @@ class ExternalStore {
         * @param array $params Map of ExternalStoreMedium::__construct context parameters
         * @return string The URL of the stored data item
         * @throws MWException
+        * @deprecated 1.34
         */
        public static function insertWithFallback( array $tryStores, $data, array $params = [] ) {
-               $error = false;
-               while ( count( $tryStores ) > 0 ) {
-                       $index = mt_rand( 0, count( $tryStores ) - 1 );
-                       $storeUrl = $tryStores[$index];
-                       wfDebug( __METHOD__ . ": trying $storeUrl\n" );
-                       list( $proto, $path ) = explode( '://', $storeUrl, 2 );
-                       $store = self::getStoreObject( $proto, $params );
-                       if ( $store === false ) {
-                               throw new MWException( "Invalid external storage protocol - $storeUrl" );
-                       }
-
-                       try {
-                               if ( $store->isReadOnly( $path ) ) {
-                                       $msg = 'read only';
-                               } else {
-                                       $url = $store->store( $path, $data );
-                                       if ( $url !== false ) {
-                                               return $url; // a store accepted the write; done!
-                                       }
-                                       $msg = 'operation failed';
-                               }
-                       } catch ( Exception $error ) {
-                               $msg = 'caught exception';
-                       }
-
-                       unset( $tryStores[$index] ); // Don't try this one again!
-                       $tryStores = array_values( $tryStores ); // Must have consecutive keys
-                       wfDebugLog( 'ExternalStorage',
-                               "Unable to store text to external storage $storeUrl ($msg)" );
-               }
-               // All stores failed
-               if ( $error ) {
-                       throw $error; // rethrow the last error
-               } else {
-                       throw new MWException( "Unable to store text to external storage" );
-               }
-       }
-
-       /**
-        * @return bool Whether all the default insertion stores are marked as read-only
-        * @since 1.31
-        */
-       public static function defaultStoresAreReadOnly() {
-               global $wgDefaultExternalStore;
-
-               $tryStores = (array)$wgDefaultExternalStore;
-               if ( !$tryStores ) {
-                       return false; // no stores exists which can be "read only"
-               }
-
-               foreach ( $tryStores as $storeUrl ) {
-                       list( $proto, $path ) = explode( '://', $storeUrl, 2 );
-                       $store = self::getStoreObject( $proto, [] );
-                       if ( !$store->isReadOnly( $path ) ) {
-                               return false; // at least one store is not read-only
-                       }
-               }
-
-               return true; // all stores are read-only
+               return MediaWikiServices::getInstance()
+                       ->getExternalStoreAccess()
+                       ->insert( $data, $params, $tryStores );
        }
 
        /**
@@ -247,8 +160,11 @@ class ExternalStore {
         * @param string $wiki
         * @return string The URL of the stored data item
         * @throws MWException
+        * @deprecated 1.34 Use insertToDefault() with 'wiki' set
         */
        public static function insertToForeignDefault( $data, $wiki ) {
-               return self::insertToDefault( $data, [ 'wiki' => $wiki ] );
+               return MediaWikiServices::getInstance()
+                       ->getExternalStoreAccess()
+                       ->insert( $data, [ 'domain' => $wiki ] );
        }
 }
diff --git a/includes/externalstore/ExternalStoreAccess.php b/includes/externalstore/ExternalStoreAccess.php
new file mode 100644 (file)
index 0000000..8603cc2
--- /dev/null
@@ -0,0 +1,164 @@
+<?php
+/**
+ * @defgroup ExternalStorage ExternalStorage
+ */
+
+use \Psr\Log\LoggerAwareInterface;
+use \Psr\Log\LoggerInterface;
+use \Psr\Log\NullLogger;
+
+/**
+ * Key/value blob storage for a collection of storage medium types (e.g. RDBMs, files)
+ *
+ * Multiple medium types can be active and each one can have multiple "locations" available.
+ * Blobs are stored under URLs of the form "<protocol>://<location>/<path>". Each type of storage
+ * medium has an associated protocol. Insertions will randomly pick mediums and locations from
+ * the provided list of writable medium-qualified locations. Insertions will also fail-over to
+ * other writable locations or mediums if one or more are not available.
+ *
+ * @ingroup ExternalStorage
+ * @since 1.34
+ */
+class ExternalStoreAccess implements LoggerAwareInterface {
+       /** @var ExternalStoreFactory */
+       private $storeFactory;
+       /** @var LoggerInterface */
+       private $logger;
+
+       /**
+        * @param ExternalStoreFactory $factory
+        * @param LoggerInterface|null $logger
+        */
+       public function __construct( ExternalStoreFactory $factory, LoggerInterface $logger = null ) {
+               $this->storeFactory = $factory;
+               $this->logger = $logger ?: new NullLogger();
+       }
+
+       public function setLogger( LoggerInterface $logger ) {
+               $this->logger = $logger;
+       }
+
+       /**
+        * Fetch data from given URL
+        *
+        * @see ExternalStoreFactory::getStore()
+        *
+        * @param string $url The URL of the text to get
+        * @param array $params Map of context parameters; same as ExternalStoreFactory::getStore()
+        * @return string|bool The text stored or false on error
+        * @throws ExternalStoreException
+        */
+       public function fetchFromURL( $url, array $params = [] ) {
+               return $this->storeFactory->getStoreForUrl( $url, $params )->fetchFromURL( $url );
+       }
+
+       /**
+        * Fetch data from multiple URLs with a minimum of round trips
+        *
+        * @see ExternalStoreFactory::getStore()
+        *
+        * @param array $urls The URLs of the text to get
+        * @param array $params Map of context parameters; same as ExternalStoreFactory::getStore()
+        * @return array Map of (url => string or false if not found)
+        * @throws ExternalStoreException
+        */
+       public function fetchFromURLs( array $urls, array $params = [] ) {
+               $batches = $this->storeFactory->getUrlsByProtocol( $urls );
+               $retval = [];
+               foreach ( $batches as $proto => $batchedUrls ) {
+                       $store = $this->storeFactory->getStore( $proto, $params );
+                       $retval += $store->batchFetchFromURLs( $batchedUrls );
+               }
+               // invalid, not found, db dead, etc.
+               $missing = array_diff( $urls, array_keys( $retval ) );
+               foreach ( $missing as $url ) {
+                       $retval[$url] = false;
+               }
+
+               return $retval;
+       }
+
+       /**
+        * Insert data into storage and return the assigned URL
+        *
+        * This will randomly pick one of the available write storage locations to put the data.
+        * It will keep failing-over to any untried storage locations whenever one location is
+        * not usable.
+        *
+        * @see ExternalStoreFactory::getStore()
+        *
+        * @param string $data
+        * @param array $params Map of context parameters; same as ExternalStoreFactory::getStore()
+        * @param string[]|null $tryStores Refer to $wgDefaultExternalStore
+        * @return string|bool The URL of the stored data item, or false on error
+        * @throws ExternalStoreException
+        */
+       public function insert( $data, array $params = [], array $tryStores = null ) {
+               $tryStores = $tryStores ?? $this->storeFactory->getWriteBaseUrls();
+               if ( !$tryStores ) {
+                       throw new ExternalStoreException( "List of external stores provided is empty." );
+               }
+
+               $error = false;
+               while ( count( $tryStores ) > 0 ) {
+                       $index = mt_rand( 0, count( $tryStores ) - 1 );
+                       $storeUrl = $tryStores[$index];
+
+                       $this->logger->debug( __METHOD__ . ": trying $storeUrl\n" );
+
+                       $store = $this->storeFactory->getStoreForUrl( $storeUrl, $params );
+                       if ( $store === false ) {
+                               throw new ExternalStoreException( "Invalid external storage protocol - $storeUrl" );
+                       }
+
+                       $location = $this->storeFactory->getStoreLocationFromUrl( $storeUrl );
+                       try {
+                               if ( $store->isReadOnly( $location ) ) {
+                                       $msg = 'read only';
+                               } else {
+                                       $url = $store->store( $location, $data );
+                                       if ( strlen( $url ) ) {
+                                               return $url; // a store accepted the write; done!
+                                       }
+                                       $msg = 'operation failed';
+                               }
+                       } catch ( Exception $error ) {
+                               $msg = 'caught ' . get_class( $error ) . ' exception: ' . $error->getMessage();
+                       }
+
+                       unset( $tryStores[$index] ); // Don't try this one again!
+                       $tryStores = array_values( $tryStores ); // Must have consecutive keys
+                       $this->logger->error(
+                               "Unable to store text to external storage {store_path} ({failure})",
+                               [ 'store_path' => $storeUrl, 'failure' => $msg ]
+                       );
+               }
+               // All stores failed
+               if ( $error ) {
+                       throw $error; // rethrow the last error
+               } else {
+                       throw new ExternalStoreException( "Unable to store text to external storage" );
+               }
+       }
+
+       /**
+        * @return bool Whether all the default insertion stores are marked as read-only
+        * @throws ExternalStoreException
+        */
+       public function isReadOnly() {
+               $writableStores = $this->storeFactory->getWriteBaseUrls();
+               if ( !$writableStores ) {
+                       return false; // no stores exists which can be "read only"
+               }
+
+               foreach ( $writableStores as $storeUrl ) {
+                       $store = $this->storeFactory->getStoreForUrl( $storeUrl );
+                       $location = $this->storeFactory->getStoreLocationFromUrl( $storeUrl );
+                       if ( $store !== false && !$store->isReadOnly( $location ) ) {
+                               return false; // at least one store is not read-only
+                       }
+               }
+
+               return true; // all stores are read-only
+       }
+}
index 15bc3e0..feb0614 100644 (file)
@@ -20,7 +20,7 @@
  * @file
  */
 
-use MediaWiki\MediaWikiServices;
+use Wikimedia\Rdbms\LBFactory;
 use Wikimedia\Rdbms\ILoadBalancer;
 use Wikimedia\Rdbms\IDatabase;
 use Wikimedia\Rdbms\DBConnRef;
@@ -36,6 +36,22 @@ use Wikimedia\Rdbms\DatabaseDomain;
  * @ingroup ExternalStorage
  */
 class ExternalStoreDB extends ExternalStoreMedium {
+       /** @var LBFactory */
+       private $lbFactory;
+
+       /**
+        * @see ExternalStoreMedium::__construct()
+        * @param array $params Additional parameters include:
+        *   - lbFactory: an LBFactory instance
+        */
+       public function __construct( array $params ) {
+               parent::__construct( $params );
+               if ( !isset( $params['lbFactory'] ) || !( $params['lbFactory'] instanceof LBFactory ) ) {
+                       throw new InvalidArgumentException( "LBFactory required in 'lbFactory' field." );
+               }
+               $this->lbFactory = $params['lbFactory'];
+       }
+
        /**
         * The provided URL is in the form of DB://cluster/id
         * or DB://cluster/id/itemid for concatened storage.
@@ -97,9 +113,7 @@ class ExternalStoreDB extends ExternalStoreMedium {
         */
        public function store( $location, $data ) {
                $dbw = $this->getMaster( $location );
-               $dbw->insert( $this->getTable( $dbw ),
-                       [ 'blob_text' => $data ],
-                       __METHOD__ );
+               $dbw->insert( $this->getTable( $dbw ), [ 'blob_text' => $data ], __METHOD__ );
                $id = $dbw->insertId();
                if ( !$id ) {
                        throw new MWException( __METHOD__ . ': no insert ID' );
@@ -112,8 +126,13 @@ class ExternalStoreDB extends ExternalStoreMedium {
         * @inheritDoc
         */
        public function isReadOnly( $location ) {
+               if ( parent::isReadOnly( $location ) ) {
+                       return true;
+               }
+
                $lb = $this->getLoadBalancer( $location );
                $domainId = $this->getDomainId( $lb->getServerInfo( $lb->getWriterIndex() ) );
+
                return ( $lb->getReadOnlyReason( $domainId ) !== false );
        }
 
@@ -124,8 +143,7 @@ class ExternalStoreDB extends ExternalStoreMedium {
         * @return ILoadBalancer
         */
        private function getLoadBalancer( $cluster ) {
-               $lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
-               return $lbFactory->getExternalLB( $cluster );
+               return $this->lbFactory->getExternalLB( $cluster );
        }
 
        /**
@@ -135,16 +153,14 @@ class ExternalStoreDB extends ExternalStoreMedium {
         * @return DBConnRef
         */
        public function getSlave( $cluster ) {
-               global $wgDefaultExternalStore;
-
                $lb = $this->getLoadBalancer( $cluster );
                $domainId = $this->getDomainId( $lb->getServerInfo( $lb->getWriterIndex() ) );
 
-               if ( !in_array( "DB://" . $cluster, (array)$wgDefaultExternalStore ) ) {
-                       wfDebug( "read only external store\n" );
+               if ( !in_array( $cluster, $this->writableLocations, true ) ) {
+                       $this->logger->debug( "read only external store\n" );
                        $lb->allowLagged( true );
                } else {
-                       wfDebug( "writable external store\n" );
+                       $this->logger->debug( "writable external store\n" );
                }
 
                $db = $lb->getConnectionRef( DB_REPLICA, [], $domainId );
@@ -174,8 +190,8 @@ class ExternalStoreDB extends ExternalStoreMedium {
         * @return string|bool Database domain ID or false
         */
        private function getDomainId( array $server ) {
-               if ( isset( $this->params['wiki'] ) && $this->params['wiki'] !== false ) {
-                       return $this->params['wiki']; // explicit domain
+               if ( $this->isDbDomainExplicit ) {
+                       return $this->dbDomain; // explicit foreign domain
                }
 
                if ( isset( $server['dbname'] ) ) {
@@ -230,33 +246,27 @@ class ExternalStoreDB extends ExternalStoreMedium {
                static $externalBlobCache = [];
 
                $cacheID = ( $itemID === false ) ? "$cluster/$id" : "$cluster/$id/";
-
-               $wiki = $this->params['wiki'] ?? false;
-               $cacheID = ( $wiki === false ) ? $cacheID : "$cacheID@$wiki";
+               $cacheID = "$cacheID@{$this->dbDomain}";
 
                if ( isset( $externalBlobCache[$cacheID] ) ) {
-                       wfDebugLog( 'ExternalStoreDB-cache',
-                               "ExternalStoreDB::fetchBlob cache hit on $cacheID" );
+                       $this->logger->debug( "ExternalStoreDB::fetchBlob cache hit on $cacheID" );
 
                        return $externalBlobCache[$cacheID];
                }
 
-               wfDebugLog( 'ExternalStoreDB-cache',
-                       "ExternalStoreDB::fetchBlob cache miss on $cacheID" );
+               $this->logger->debug( "ExternalStoreDB::fetchBlob cache miss on $cacheID" );
 
                $dbr = $this->getSlave( $cluster );
                $ret = $dbr->selectField( $this->getTable( $dbr ),
                        'blob_text', [ 'blob_id' => $id ], __METHOD__ );
                if ( $ret === false ) {
-                       wfDebugLog( 'ExternalStoreDB',
-                               "ExternalStoreDB::fetchBlob master fallback on $cacheID" );
+                       $this->logger->info( "ExternalStoreDB::fetchBlob master fallback on $cacheID" );
                        // Try the master
                        $dbw = $this->getMaster( $cluster );
                        $ret = $dbw->selectField( $this->getTable( $dbw ),
                                'blob_text', [ 'blob_id' => $id ], __METHOD__ );
                        if ( $ret === false ) {
-                               wfDebugLog( 'ExternalStoreDB',
-                                       "ExternalStoreDB::fetchBlob master failed to find $cacheID" );
+                               $this->logger->error( "ExternalStoreDB::fetchBlob master failed to find $cacheID" );
                        }
                }
                if ( $itemID !== false && $ret !== false ) {
@@ -279,16 +289,22 @@ class ExternalStoreDB extends ExternalStoreMedium {
         */
        private function batchFetchBlobs( $cluster, array $ids ) {
                $dbr = $this->getSlave( $cluster );
-               $res = $dbr->select( $this->getTable( $dbr ),
-                       [ 'blob_id', 'blob_text' ], [ 'blob_id' => array_keys( $ids ) ], __METHOD__ );
+               $res = $dbr->select(
+                       $this->getTable( $dbr ),
+                       [ 'blob_id', 'blob_text' ],
+                       [ 'blob_id' => array_keys( $ids ) ],
+                       __METHOD__
+               );
+
                $ret = [];
                if ( $res !== false ) {
                        $this->mergeBatchResult( $ret, $ids, $res );
                }
                if ( $ids ) {
-                       wfDebugLog( __CLASS__, __METHOD__ .
-                               " master fallback on '$cluster' for: " .
-                               implode( ',', array_keys( $ids ) ) );
+                       $this->logger->info(
+                               __METHOD__ . ": master fallback on '$cluster' for: " .
+                               implode( ',', array_keys( $ids ) )
+                       );
                        // Try the master
                        $dbw = $this->getMaster( $cluster );
                        $res = $dbw->select( $this->getTable( $dbr ),
@@ -296,15 +312,16 @@ class ExternalStoreDB extends ExternalStoreMedium {
                                [ 'blob_id' => array_keys( $ids ) ],
                                __METHOD__ );
                        if ( $res === false ) {
-                               wfDebugLog( __CLASS__, __METHOD__ . " master failed on '$cluster'" );
+                               $this->logger->error( __METHOD__ . ": master failed on '$cluster'" );
                        } else {
                                $this->mergeBatchResult( $ret, $ids, $res );
                        }
                }
                if ( $ids ) {
-                       wfDebugLog( __CLASS__, __METHOD__ .
-                               " master on '$cluster' failed locating items: " .
-                               implode( ',', array_keys( $ids ) ) );
+                       $this->logger->error(
+                               __METHOD__ . ": master on '$cluster' failed locating items: " .
+                               implode( ',', array_keys( $ids ) )
+                       );
                }
 
                return $ret;
diff --git a/includes/externalstore/ExternalStoreException.php b/includes/externalstore/ExternalStoreException.php
new file mode 100644 (file)
index 0000000..a2ef27d
--- /dev/null
@@ -0,0 +1,5 @@
+<?php
+
+class ExternalStoreException extends MWException {
+
+}
index 940fb2e..3f78b8b 100644 (file)
  * @defgroup ExternalStorage ExternalStorage
  */
 
+use MediaWiki\MediaWikiServices;
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
+use Wikimedia\Assert\Assert;
+
 /**
  * @ingroup ExternalStorage
  */
-class ExternalStoreFactory {
+class ExternalStoreFactory implements LoggerAwareInterface {
+       /** @var string[] List of storage access protocols */
+       private $protocols;
+       /** @var string[] List of base storage URLs that define locations for writes */
+       private $writeBaseUrls;
+       /** @var string Default database domain to store content under */
+       private $localDomainId;
+       /** @var LoggerInterface */
+       private $logger;
 
        /**
-        * @var array
+        * @param string[] $externalStores See $wgExternalStores
+        * @param string[] $defaultStores See $wgDefaultExternalStore
+        * @param string $localDomainId Local database/wiki ID
+        * @param LoggerInterface|null $logger
         */
-       private $externalStores;
+       public function __construct(
+               array $externalStores,
+               array $defaultStores,
+               $localDomainId,
+               LoggerInterface $logger = null
+       ) {
+               Assert::parameterType( 'string', $localDomainId, '$localDomainId' );
+
+               $this->protocols = array_map( 'strtolower', $externalStores );
+               $this->writeBaseUrls = $defaultStores;
+               $this->localDomainId = $localDomainId;
+               $this->logger = $logger ?: new NullLogger();
+       }
+
+       public function setLogger( LoggerInterface $logger ) {
+               $this->logger = $logger;
+       }
 
        /**
-        * @param array $externalStores See $wgExternalStores
+        * @return string[] List of active store types/protocols (lowercased), e.g. [ "db" ]
+        * @since 1.34
         */
-       public function __construct( array $externalStores ) {
-               $this->externalStores = array_map( 'strtolower', $externalStores );
+       public function getProtocols() {
+               return $this->protocols;
+       }
+
+       /**
+        * @return string[] List of base URLs for writes, e.g. [ "DB://cluster1" ]
+        * @since 1.34
+        */
+       public function getWriteBaseUrls() {
+               return $this->writeBaseUrls;
        }
 
        /**
         * Get an external store object of the given type, with the given parameters
         *
+        * The 'domain' field in $params will be set to the local DB domain if it is unset
+        * or false. A special 'isDomainImplicit' flag is set when this happens, which should
+        * only be used to handle legacy DB domain configuration concerns (e.g. T200471).
+        *
         * @param string $proto Type of external storage, should be a value in $wgExternalStores
-        * @param array $params Associative array of ExternalStoreMedium parameters
-        * @return ExternalStoreMedium|bool The store class or false on error
+        * @param array $params Map of ExternalStoreMedium::__construct context parameters.
+        * @return ExternalStoreMedium The store class or false on error
+        * @throws ExternalStoreException When $proto is not recognized
         */
-       public function getStoreObject( $proto, array $params = [] ) {
-               if ( !$this->externalStores || !in_array( strtolower( $proto ), $this->externalStores ) ) {
-                       // Protocol not enabled
-                       return false;
+       public function getStore( $proto, array $params = [] ) {
+               $protoLowercase = strtolower( $proto ); // normalize
+               if ( !$this->protocols || !in_array( $protoLowercase, $this->protocols ) ) {
+                       throw new ExternalStoreException( "Protocol '$proto' is not enabled." );
                }
 
                $class = 'ExternalStore' . ucfirst( $proto );
+               if ( isset( $params['wiki'] ) ) {
+                       $params += [ 'domain' => $params['wiki'] ]; // b/c
+               }
+               if ( !isset( $params['domain'] ) || $params['domain'] === false ) {
+                       $params['domain'] = $this->localDomainId; // default
+                       $params['isDomainImplicit'] = true; // b/c for ExternalStoreDB
+               }
+               $params['writableLocations'] = [];
+               // Determine the locations for this protocol/store still receiving writes
+               foreach ( $this->writeBaseUrls as $storeUrl ) {
+                       list( $storeProto, $storePath ) = self::splitStorageUrl( $storeUrl );
+                       if ( $protoLowercase === strtolower( $storeProto ) ) {
+                               $params['writableLocations'][] = $storePath;
+                       }
+               }
+               // @TODO: ideally, this class should not hardcode what classes need what backend factory
+               // objects. For now, inject the factory instances into __construct() for those that do.
+               if ( $protoLowercase === 'db' ) {
+                       $params['lbFactory'] = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
+               } elseif ( $protoLowercase === 'mwstore' ) {
+                       $params['fbGroup'] = FileBackendGroup::singleton();
+               }
+               $params['logger'] = $this->logger;
+
+               if ( !class_exists( $class ) ) {
+                       throw new ExternalStoreException( "Class '$class' is not defined." );
+               }
 
                // Any custom modules should be added to $wgAutoLoadClasses for on-demand loading
-               return class_exists( $class ) ? new $class( $params ) : false;
+               return new $class( $params );
+       }
+
+       /**
+        * Get the ExternalStoreMedium for a given URL
+        *
+        * $url is either of the form:
+        *   - a) "<proto>://<location>/<path>", for retrieval, or
+        *   - b) "<proto>://<location>", for storage
+        *
+        * @param string $url
+        * @param array $params Map of ExternalStoreMedium::__construct context parameters
+        * @return ExternalStoreMedium
+        * @throws ExternalStoreException When the protocol is missing or not recognized
+        * @since 1.34
+        */
+       public function getStoreForUrl( $url, array $params = [] ) {
+               list( $proto, $path ) = self::splitStorageUrl( $url );
+               if ( $path == '' ) { // bad URL
+                       throw new ExternalStoreException( "Invalid URL '$url'" );
+               }
+
+               return $this->getStore( $proto, $params );
        }
 
+       /**
+        * Get the location within the appropriate store for a given a URL
+        *
+        * @param string $url
+        * @return string
+        * @throws ExternalStoreException
+        * @since 1.34
+        */
+       public function getStoreLocationFromUrl( $url ) {
+               list( , $location ) = self::splitStorageUrl( $url );
+               if ( $location == '' ) { // bad URL
+                       throw new ExternalStoreException( "Invalid URL '$url'" );
+               }
+
+               return $location;
+       }
+
+       /**
+        * @param string[] $urls
+        * @return array[] Map of (protocol => list of URLs)
+        * @throws ExternalStoreException
+        * @since 1.34
+        */
+       public function getUrlsByProtocol( array $urls ) {
+               $urlsByProtocol = [];
+               foreach ( $urls as $url ) {
+                       list( $proto, ) = self::splitStorageUrl( $url );
+                       $urlsByProtocol[$proto][] = $url;
+               }
+
+               return $urlsByProtocol;
+       }
+
+       /**
+        * @param string $storeUrl
+        * @return string[] (protocol, store location or location-qualified path)
+        * @throws ExternalStoreException
+        */
+       private static function splitStorageUrl( $storeUrl ) {
+               $parts = explode( '://', $storeUrl );
+               if ( count( $parts ) != 2 || $parts[0] === '' || $parts[1] === '' ) {
+                       throw new ExternalStoreException( "Invalid storage URL '$storeUrl'" );
+               }
+
+               return $parts;
+       }
 }
index da7752b..0cdcf53 100644 (file)
  * @ingroup ExternalStorage
  */
 
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
+
 /**
- * Accessable external objects in a particular storage medium
+ * Key/value blob storage for a particular storage medium type (e.g. RDBMs, files)
+ *
+ * There can be multiple "locations" for a storage medium type (e.g. DB clusters, filesystems).
+ * Blobs are stored under URLs of the form "<protocol>://<location>/<path>". Each type of storage
+ * medium has an associated protocol.
  *
  * @ingroup ExternalStorage
  * @since 1.21
  */
-abstract class ExternalStoreMedium {
-       /** @var array */
+abstract class ExternalStoreMedium implements LoggerAwareInterface {
+       /** @var array Usage context options for this instance */
        protected $params = [];
+       /** @var string Default database domain to store content under */
+       protected $dbDomain;
+       /** @var bool Whether this was factoried with an explicit DB domain */
+       protected $isDbDomainExplicit;
+       /** @var string[] Writable locations */
+       protected $writableLocations = [];
+
+       /** @var LoggerInterface */
+       protected $logger;
 
        /**
-        * @param array $params Usage context options:
-        *   - wiki: the domain ID of the wiki this is being used for [optional]
+        * @param array $params Usage context options for this instance:
+        *   - domain: the DB domain ID of the wiki the content is for [required]
+        *   - writableLocations: locations that are writable [required]
+        *   - logger: LoggerInterface instance [optional]
+        *   - isDomainImplicit: whether this was factoried without an explicit DB domain [optional]
         */
-       public function __construct( array $params = [] ) {
+       public function __construct( array $params ) {
                $this->params = $params;
+               if ( isset( $params['domain'] ) ) {
+                       $this->dbDomain = $params['domain'];
+                       $this->isDbDomainExplicit = empty( $params['isDomainImplicit'] );
+               } else {
+                       throw new InvalidArgumentException( 'Missing DB "domain" parameter.' );
+               }
+
+               $this->logger = $params['logger'] ?? new NullLogger();
+               $this->writableLocations = $params['writableLocations'] ?? [];
+       }
+
+       public function setLogger( LoggerInterface $logger ) {
+               $this->logger = $logger;
        }
 
        /**
@@ -52,14 +85,13 @@ abstract class ExternalStoreMedium {
         * Fetch data from given external store URLs.
         *
         * @param array $urls A list of external store URLs
-        * @return array Map from the url to the text stored. Unfound data is not represented
+        * @return string[] Map of (url => text) for the URLs where data was actually found
         */
        public function batchFetchFromURLs( array $urls ) {
                $retval = [];
                foreach ( $urls as $url ) {
                        $data = $this->fetchFromURL( $url );
-                       // Dont return when false to allow for simpler implementations.
-                       // errored urls are handled in ExternalStore::batchFetchFromURLs
+                       // Dont return when false to allow for simpler implementations
                        if ( $data !== false ) {
                                $retval[$url] = $data;
                        }
@@ -86,6 +118,6 @@ abstract class ExternalStoreMedium {
         * @since 1.31
         */
        public function isReadOnly( $location ) {
-               return false;
+               return !in_array( $location, $this->writableLocations, true );
        }
 }
diff --git a/includes/externalstore/ExternalStoreMemory.php b/includes/externalstore/ExternalStoreMemory.php
new file mode 100644 (file)
index 0000000..daad75c
--- /dev/null
@@ -0,0 +1,102 @@
+<?php
+/**
+ * External storage in PHP process memory for testing.
+ *
+ * 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
+ *
+ * @file
+ */
+
+/**
+ * Process memory based external objects for testing.
+ *
+ * In this system, each store "location" is separate PHP array.
+ * URLs are of the form "memory://location/id". The id/value pairs
+ * at each location are segregated by DB domain ID.
+ *
+ * @ingroup ExternalStorage
+ * @since 1.33
+ */
+class ExternalStoreMemory extends ExternalStoreMedium {
+       /** @var array[] Map of (location => DB domain => id => value) */
+       private static $data = [];
+       /** @var int */
+       private static $nextId = 0;
+
+       public function __construct( array $params ) {
+               parent::__construct( $params );
+       }
+
+       public function fetchFromURL( $url ) {
+               list( $location, $id ) = self::getURLComponents( $url );
+               if ( $id === null ) {
+                       throw new UnexpectedValueException( "Missing ID in URL component." );
+               }
+
+               return self::$data[$location][$this->dbDomain][$id] ?? false;
+       }
+
+       public function batchFetchFromURLs( array $urls ) {
+               $blobs = [];
+               foreach ( $urls as $url ) {
+                       $blob = $this->fetchFromURL( $url );
+                       if ( $blob !== false ) {
+                               $blobs[$url] = $blob;
+                       }
+               }
+
+               return $blobs;
+       }
+
+       public function store( $location, $data ) {
+               $index = ++self::$nextId;
+               self::$data[$location][$this->dbDomain][$index] = $data;
+
+               return "memory://$location/$index";
+       }
+
+       /**
+        * Remove all data from memory for this domain
+        */
+       public function clear() {
+               foreach ( self::$data as &$dataForLocation ) {
+                       unset( $dataForLocation[$this->dbDomain] );
+               }
+               unset( $dataForLocation );
+               self::$data = array_filter( self::$data, 'count' );
+               self::$nextId = 0;
+       }
+
+       /**
+        * @param string $url
+        * @return array (location, ID or null)
+        */
+       private function getURLComponents( $url ) {
+               list( $proto, $path ) = explode( '://', $url, 2 ) + [ null, null ];
+               if ( $proto !== 'memory' ) {
+                       throw new UnexpectedValueException( "Got URL of protocol '$proto', not 'memory'." );
+               } elseif ( $path === null ) {
+                       throw new UnexpectedValueException( "URL is missing path component." );
+               }
+
+               $parts = explode( '/', $path );
+               if ( count( $parts ) > 2 ) {
+                       throw new UnexpectedValueException( "Too components in URL '$path'." );
+               }
+
+               return [ $parts[0], $parts[1] ?? null ];
+       }
+}
index 7414f23..77c23ee 100644 (file)
  * @since 1.21
  */
 class ExternalStoreMwstore extends ExternalStoreMedium {
+       /** @var FileBackendGroup */
+       private $fbGroup;
+
+       /**
+        * @see ExternalStoreMedium::__construct()
+        * @param array $params Additional parameters include:
+        *   - fbGroup: a FileBackendGroup instance
+        */
+       public function __construct( array $params ) {
+               parent::__construct( $params );
+               if ( !isset( $params['fbGroup'] ) || !( $params['fbGroup'] instanceof FileBackendGroup ) ) {
+                       throw new InvalidArgumentException( "FileBackendGroup required in 'fbGroup' field." );
+               }
+               $this->fbGroup = $params['fbGroup'];
+       }
+
        /**
         * The URL returned is of the form of the form mwstore://backend/container/wiki/id
         *
@@ -39,7 +55,7 @@ class ExternalStoreMwstore extends ExternalStoreMedium {
         * @return bool
         */
        public function fetchFromURL( $url ) {
-               $be = FileBackendGroup::singleton()->backendFromPath( $url );
+               $be = $this->fbGroup->backendFromPath( $url );
                if ( $be instanceof FileBackend ) {
                        // We don't need "latest" since objects are immutable and
                        // backends should at least have "read-after-create" consistency.
@@ -59,14 +75,14 @@ class ExternalStoreMwstore extends ExternalStoreMedium {
        public function batchFetchFromURLs( array $urls ) {
                $pathsByBackend = [];
                foreach ( $urls as $url ) {
-                       $be = FileBackendGroup::singleton()->backendFromPath( $url );
+                       $be = $this->fbGroup->backendFromPath( $url );
                        if ( $be instanceof FileBackend ) {
                                $pathsByBackend[$be->getName()][] = $url;
                        }
                }
                $blobs = [];
                foreach ( $pathsByBackend as $backendName => $paths ) {
-                       $be = FileBackendGroup::singleton()->get( $backendName );
+                       $be = $this->fbGroup->get( $backendName );
                        $blobs += $be->getFileContentsMulti( [ 'srcs' => $paths ] );
                }
 
@@ -77,16 +93,18 @@ class ExternalStoreMwstore extends ExternalStoreMedium {
         * @inheritDoc
         */
        public function store( $backend, $data ) {
-               $be = FileBackendGroup::singleton()->get( $backend );
+               $be = $this->fbGroup->get( $backend );
                // Get three random base 36 characters to act as shard directories
                $rand = Wikimedia\base_convert( mt_rand( 0, 46655 ), 10, 36, 3 );
                // 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 );
+               // Segregate items by DB domain ID for the sake of bookkeeping
+               $domain = $this->isDbDomainExplicit
+                       ? $this->dbDomain
+                       // @FIXME: this does not include the schema for b/c but it ideally should
+                       : WikiMap::getWikiIdFromDbDomain( $this->dbDomain );
+               $url = $be->getContainerStoragePath( 'data' ) . '/' . rawurlencode( $domain );
+               // Use directory/container sharding
                $url .= ( $be instanceof FSFileBackend )
                        ? "/{$rand[0]}/{$rand[1]}/{$rand[2]}/{$id}" // keep directories small
                        : "/{$rand[0]}/{$rand[1]}/{$id}"; // container sharding is only 2-levels
@@ -96,13 +114,17 @@ class ExternalStoreMwstore extends ExternalStoreMedium {
 
                if ( $status->isOK() ) {
                        return $url;
-               } else {
-                       throw new MWException( __METHOD__ . ": operation failed: $status" );
                }
+
+               throw new MWException( __METHOD__ . ": operation failed: $status" );
        }
 
        public function isReadOnly( $backend ) {
-               $be = FileBackendGroup::singleton()->get( $backend );
+               if ( parent::isReadOnly( $backend ) ) {
+                       return true;
+               }
+
+               $be = $this->fbGroup->get( $backend );
 
                return $be ? $be->isReadOnly() : false;
        }
index 4995d3b..9a4df1f 100644 (file)
@@ -20,6 +20,8 @@
  * @file
  */
 
+use MediaWiki\MediaWikiServices;
+
 /**
  * Pointer object for an item within a CGZ blob stored in the text table.
  */
@@ -99,8 +101,9 @@ class HistoryBlobStub {
                                if ( !isset( $parts[1] ) || $parts[1] == '' ) {
                                        return false;
                                }
-                               $row->old_text = ExternalStore::fetchFromURL( $url );
-
+                               $row->old_text = MediaWikiServices::getInstance()
+                                       ->getExternalStoreAccess()
+                                       ->fetchFromURL( $url );
                        }
 
                        if ( !in_array( 'object', $flags ) ) {
index dc5aa22..a1b2460 100644 (file)
@@ -297,7 +297,7 @@ class SwiftFileBackend extends FileBackendStore {
                $method = __METHOD__;
                $handler = function ( array $request, StatusValue $status ) use ( $method, $params ) {
                        list( $rcode, $rdesc, $rhdrs, $rbody, $rerr ) = $request['response'];
-                       if ( $rcode === 201 ) {
+                       if ( $rcode === 201 || $rcode === 202 ) {
                                // good
                        } elseif ( $rcode === 412 ) {
                                $status->fatal( 'backend-fail-contenttype', $params['dst'] );
@@ -360,7 +360,7 @@ class SwiftFileBackend extends FileBackendStore {
                $method = __METHOD__;
                $handler = function ( array $request, StatusValue $status ) use ( $method, $params ) {
                        list( $rcode, $rdesc, $rhdrs, $rbody, $rerr ) = $request['response'];
-                       if ( $rcode === 201 ) {
+                       if ( $rcode === 201 || $rcode === 202 ) {
                                // good
                        } elseif ( $rcode === 412 ) {
                                $status->fatal( 'backend-fail-contenttype', $params['dst'] );
index 27e6138..50a0b0e 100644 (file)
@@ -73,7 +73,7 @@ class ConnectionManager {
         * @param int $i
         * @param string[]|null $groups
         *
-        * @return Database
+        * @return IDatabase
         */
        private function getConnection( $i, array $groups = null ) {
                $groups = $groups === null ? $this->groups : $groups;
@@ -97,7 +97,7 @@ class ConnectionManager {
         *
         * @since 1.29
         *
-        * @return Database
+        * @return IDatabase
         */
        public function getWriteConnection() {
                return $this->getConnection( DB_MASTER );
@@ -111,7 +111,7 @@ class ConnectionManager {
         *
         * @param string[]|null $groups
         *
-        * @return Database
+        * @return IDatabase
         */
        public function getReadConnection( array $groups = null ) {
                $groups = $groups === null ? $this->groups : $groups;
index aa3bea8..ccb73d7 100644 (file)
@@ -64,7 +64,7 @@ class SessionConsistentConnectionManager extends ConnectionManager {
         *
         * @param string[]|null $groups
         *
-        * @return Database
+        * @return IDatabase
         */
        public function getReadConnection( array $groups = null ) {
                if ( $this->forceWriteConnection ) {
@@ -77,7 +77,7 @@ class SessionConsistentConnectionManager extends ConnectionManager {
        /**
         * @since 1.29
         *
-        * @return Database
+        * @return IDatabase
         */
        public function getWriteConnection() {
                $this->prepareForUpdates();
index c86adac..760d137 100644 (file)
@@ -105,9 +105,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        /** @var array Map of (table name => 1) for TEMPORARY tables */
        protected $sessionTempTables = [];
 
-       /** @var int Whether there is an active transaction (1 or 0) */
-       protected $trxLevel = 0;
-       /** @var string Hexidecimal string if a transaction is active or empty string otherwise */
+       /** @var string ID of the active transaction or the empty string otherwise */
        protected $trxShortId = '';
        /** @var int Transaction status */
        protected $trxStatus = self::STATUS_TRX_NONE;
@@ -512,12 +510,12 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                return $res;
        }
 
-       public function trxLevel() {
-               return $this->trxLevel;
+       final public function trxLevel() {
+               return ( $this->trxShortId != '' ) ? 1 : 0;
        }
 
        public function trxTimestamp() {
-               return $this->trxLevel ? $this->trxTimestamp : null;
+               return $this->trxLevel() ? $this->trxTimestamp : null;
        }
 
        /**
@@ -620,11 +618,11 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        public function writesPending() {
-               return $this->trxLevel && $this->trxDoneWrites;
+               return $this->trxLevel() && $this->trxDoneWrites;
        }
 
        public function writesOrCallbacksPending() {
-               return $this->trxLevel && (
+               return $this->trxLevel() && (
                        $this->trxDoneWrites ||
                        $this->trxIdleCallbacks ||
                        $this->trxPreCommitCallbacks ||
@@ -634,7 +632,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        public function preCommitCallbacksPending() {
-               return $this->trxLevel && $this->trxPreCommitCallbacks;
+               return $this->trxLevel() && $this->trxPreCommitCallbacks;
        }
 
        /**
@@ -652,7 +650,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        public function pendingWriteQueryDuration( $type = self::ESTIMATE_TOTAL ) {
-               if ( !$this->trxLevel ) {
+               if ( !$this->trxLevel() ) {
                        return false;
                } elseif ( !$this->trxDoneWrites ) {
                        return 0.0;
@@ -682,7 +680,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        public function pendingWriteCallers() {
-               return $this->trxLevel ? $this->trxWriteCallers : [];
+               return $this->trxLevel() ? $this->trxWriteCallers : [];
        }
 
        public function pendingWriteRowsAffected() {
@@ -871,7 +869,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                // This should mostly do nothing if the connection is already closed
                if ( $this->conn ) {
                        // Roll back any dangling transaction first
-                       if ( $this->trxLevel ) {
+                       if ( $this->trxLevel() ) {
                                if ( $this->trxAtomicLevels ) {
                                        // Cannot let incomplete atomic sections be committed
                                        $levels = $this->flatAtomicSectionList();
@@ -1158,7 +1156,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        final protected function executeQuery( $sql, $fname, $flags ) {
                $this->assertHasConnectionHandle();
 
-               $priorTransaction = $this->trxLevel;
+               $priorTransaction = $this->trxLevel();
 
                if ( $this->isWriteQuery( $sql ) ) {
                        # In theory, non-persistent writes are allowed in read-only mode, but due to things
@@ -1248,7 +1246,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                // Keep track of whether the transaction has write queries pending
                if ( $isPermWrite ) {
                        $this->lastWriteTime = microtime( true );
-                       if ( $this->trxLevel && !$this->trxDoneWrites ) {
+                       if ( $this->trxLevel() && !$this->trxDoneWrites ) {
                                $this->trxDoneWrites = true;
                                $this->trxProfiler->transactionWritingIn(
                                        $this->server, $this->getDomainID(), $this->trxShortId );
@@ -1278,7 +1276,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 
                if ( $ret !== false ) {
                        $this->lastPing = $startTime;
-                       if ( $isPermWrite && $this->trxLevel ) {
+                       if ( $isPermWrite && $this->trxLevel() ) {
                                $this->updateTrxWriteQueryTime( $sql, $queryRuntime, $this->affectedRows() );
                                $this->trxWriteCallers[] = $fname;
                        }
@@ -1327,7 +1325,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         */
        private function beginIfImplied( $sql, $fname ) {
                if (
-                       !$this->trxLevel &&
+                       !$this->trxLevel() &&
                        $this->getFlag( self::DBO_TRX ) &&
                        $this->isTransactableQuery( $sql )
                ) {
@@ -1457,7 +1455,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                // https://www.postgresql.org/docs/9.4/static/functions-admin.html#FUNCTIONS-ADVISORY-LOCKS
                $this->sessionNamedLocks = [];
                // Session loss implies transaction loss
-               $this->trxLevel = 0;
+               $oldTrxShortId = $this->consumeTrxShortId();
                $this->trxAtomicCounter = 0;
                $this->trxIdleCallbacks = []; // T67263; transaction already lost
                $this->trxPreCommitCallbacks = []; // T67263; transaction already lost
@@ -1466,7 +1464,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        $this->trxProfiler->transactionWritingOut(
                                $this->server,
                                $this->getDomainID(),
-                               $this->trxShortId,
+                               $oldTrxShortId,
                                $this->pendingWriteQueryDuration( self::ESTIMATE_TOTAL ),
                                $this->trxWriteAffectedRows
                        );
@@ -1492,6 +1490,18 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                }
        }
 
+       /**
+        * Reset the transaction ID and return the old one
+        *
+        * @return string The old transaction ID or the empty string if there wasn't one
+        */
+       private function consumeTrxShortId() {
+               $old = $this->trxShortId;
+               $this->trxShortId = '';
+
+               return $old;
+       }
+
        /**
         * Checks whether the cause of the error is detected to be a timeout.
         *
@@ -1989,7 +1999,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        public function lockForUpdate(
                $table, $conds = '', $fname = __METHOD__, $options = [], $join_conds = []
        ) {
-               if ( !$this->trxLevel && !$this->getFlag( self::DBO_TRX ) ) {
+               if ( !$this->trxLevel() && !$this->getFlag( self::DBO_TRX ) ) {
                        throw new DBUnexpectedError(
                                $this,
                                __METHOD__ . ': no transaction is active nor is DBO_TRX set'
@@ -3336,21 +3346,21 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        final public function onTransactionResolution( callable $callback, $fname = __METHOD__ ) {
-               if ( !$this->trxLevel ) {
+               if ( !$this->trxLevel() ) {
                        throw new DBUnexpectedError( $this, "No transaction is active." );
                }
                $this->trxEndCallbacks[] = [ $callback, $fname, $this->currentAtomicSectionId() ];
        }
 
        final public function onTransactionCommitOrIdle( callable $callback, $fname = __METHOD__ ) {
-               if ( !$this->trxLevel && $this->getTransactionRoundId() ) {
+               if ( !$this->trxLevel() && $this->getTransactionRoundId() ) {
                        // Start an implicit transaction similar to how query() does
                        $this->begin( __METHOD__, self::TRANSACTION_INTERNAL );
                        $this->trxAutomatic = true;
                }
 
                $this->trxIdleCallbacks[] = [ $callback, $fname, $this->currentAtomicSectionId() ];
-               if ( !$this->trxLevel ) {
+               if ( !$this->trxLevel() ) {
                        $this->runOnTransactionIdleCallbacks( self::TRIGGER_IDLE );
                }
        }
@@ -3360,13 +3370,13 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        final public function onTransactionPreCommitOrIdle( callable $callback, $fname = __METHOD__ ) {
-               if ( !$this->trxLevel && $this->getTransactionRoundId() ) {
+               if ( !$this->trxLevel() && $this->getTransactionRoundId() ) {
                        // Start an implicit transaction similar to how query() does
                        $this->begin( __METHOD__, self::TRANSACTION_INTERNAL );
                        $this->trxAutomatic = true;
                }
 
-               if ( $this->trxLevel ) {
+               if ( $this->trxLevel() ) {
                        $this->trxPreCommitCallbacks[] = [ $callback, $fname, $this->currentAtomicSectionId() ];
                } else {
                        // No transaction is active nor will start implicitly, so make one for this callback
@@ -3382,7 +3392,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        final public function onAtomicSectionCancel( callable $callback, $fname = __METHOD__ ) {
-               if ( !$this->trxLevel || !$this->trxAtomicLevels ) {
+               if ( !$this->trxLevel() || !$this->trxAtomicLevels ) {
                        throw new DBUnexpectedError( $this, "No atomic section is open (got $fname)." );
                }
                $this->trxSectionCancelCallbacks[] = [ $callback, $fname, $this->currentAtomicSectionId() ];
@@ -3392,7 +3402,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         * @return AtomicSectionIdentifier|null ID of the topmost atomic section level
         */
        private function currentAtomicSectionId() {
-               if ( $this->trxLevel && $this->trxAtomicLevels ) {
+               if ( $this->trxLevel() && $this->trxAtomicLevels ) {
                        $levelInfo = end( $this->trxAtomicLevels );
 
                        return $levelInfo[1];
@@ -3518,7 +3528,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         * @throws Exception
         */
        public function runOnTransactionIdleCallbacks( $trigger ) {
-               if ( $this->trxLevel ) { // sanity
+               if ( $this->trxLevel() ) { // sanity
                        throw new DBUnexpectedError( $this, __METHOD__ . ': a transaction is still open.' );
                }
 
@@ -3749,7 +3759,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        ) {
                $savepointId = $cancelable === self::ATOMIC_CANCELABLE ? self::$NOT_APPLICABLE : null;
 
-               if ( !$this->trxLevel ) {
+               if ( !$this->trxLevel() ) {
                        $this->begin( $fname, self::TRANSACTION_INTERNAL ); // sets trxAutomatic
                        // If DBO_TRX is set, a series of startAtomic/endAtomic pairs will result
                        // in all changes being in one transaction to keep requests transactional.
@@ -3775,7 +3785,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        final public function endAtomic( $fname = __METHOD__ ) {
-               if ( !$this->trxLevel || !$this->trxAtomicLevels ) {
+               if ( !$this->trxLevel() || !$this->trxAtomicLevels ) {
                        throw new DBUnexpectedError( $this, "No atomic section is open (got $fname)." );
                }
 
@@ -3811,7 +3821,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        final public function cancelAtomic(
                $fname = __METHOD__, AtomicSectionIdentifier $sectionId = null
        ) {
-               if ( !$this->trxLevel || !$this->trxAtomicLevels ) {
+               if ( !$this->trxLevel() || !$this->trxAtomicLevels ) {
                        throw new DBUnexpectedError( $this, "No atomic section is open (got $fname)." );
                }
 
@@ -3916,7 +3926,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                }
 
                // Protect against mismatched atomic section, transaction nesting, and snapshot loss
-               if ( $this->trxLevel ) {
+               if ( $this->trxLevel() ) {
                        if ( $this->trxAtomicLevels ) {
                                $levels = $this->flatAtomicSectionList();
                                $msg = "$fname: Got explicit BEGIN while atomic section(s) $levels are open.";
@@ -3936,6 +3946,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                $this->assertHasConnectionHandle();
 
                $this->doBegin( $fname );
+               $this->trxShortId = sprintf( '%06x', mt_rand( 0, 0xffffff ) );
                $this->trxStatus = self::STATUS_TRX_OK;
                $this->trxStatusIgnoredCause = null;
                $this->trxAtomicCounter = 0;
@@ -3944,7 +3955,6 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                $this->trxDoneWrites = false;
                $this->trxAutomaticAtomic = false;
                $this->trxAtomicLevels = [];
-               $this->trxShortId = sprintf( '%06x', mt_rand( 0, 0xffffff ) );
                $this->trxWriteDuration = 0.0;
                $this->trxWriteQueryCount = 0;
                $this->trxWriteAffectedRows = 0;
@@ -3966,10 +3976,10 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         *
         * @see Database::begin()
         * @param string $fname
+        * @throws DBError
         */
        protected function doBegin( $fname ) {
                $this->query( 'BEGIN', $fname );
-               $this->trxLevel = 1;
        }
 
        final public function commit( $fname = __METHOD__, $flush = self::FLUSHING_ONE ) {
@@ -3978,7 +3988,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        throw new DBUnexpectedError( $this, "$fname: invalid flush parameter '$flush'." );
                }
 
-               if ( $this->trxLevel && $this->trxAtomicLevels ) {
+               if ( $this->trxLevel() && $this->trxAtomicLevels ) {
                        // There are still atomic sections open; this cannot be ignored
                        $levels = $this->flatAtomicSectionList();
                        throw new DBUnexpectedError(
@@ -3988,7 +3998,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                }
 
                if ( $flush === self::FLUSHING_INTERNAL || $flush === self::FLUSHING_ALL_PEERS ) {
-                       if ( !$this->trxLevel ) {
+                       if ( !$this->trxLevel() ) {
                                return; // nothing to do
                        } elseif ( !$this->trxAutomatic ) {
                                throw new DBUnexpectedError(
@@ -3996,7 +4006,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                                        "$fname: Flushing an explicit transaction, getting out of sync."
                                );
                        }
-               } elseif ( !$this->trxLevel ) {
+               } elseif ( !$this->trxLevel() ) {
                        $this->queryLogger->error(
                                "$fname: No transaction to commit, something got out of sync." );
                        return; // nothing to do
@@ -4013,6 +4023,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 
                $writeTime = $this->pendingWriteQueryDuration( self::ESTIMATE_DB_APPLY );
                $this->doCommit( $fname );
+               $oldTrxShortId = $this->consumeTrxShortId();
                $this->trxStatus = self::STATUS_TRX_NONE;
 
                if ( $this->trxDoneWrites ) {
@@ -4020,7 +4031,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        $this->trxProfiler->transactionWritingOut(
                                $this->server,
                                $this->getDomainID(),
-                               $this->trxShortId,
+                               $oldTrxShortId,
                                $writeTime,
                                $this->trxWriteAffectedRows
                        );
@@ -4038,16 +4049,16 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         *
         * @see Database::commit()
         * @param string $fname
+        * @throws DBError
         */
        protected function doCommit( $fname ) {
-               if ( $this->trxLevel ) {
+               if ( $this->trxLevel() ) {
                        $this->query( 'COMMIT', $fname );
-                       $this->trxLevel = 0;
                }
        }
 
        final public function rollback( $fname = __METHOD__, $flush = self::FLUSHING_ONE ) {
-               $trxActive = $this->trxLevel;
+               $trxActive = $this->trxLevel();
 
                if ( $flush !== self::FLUSHING_INTERNAL
                        && $flush !== self::FLUSHING_ALL_PEERS
@@ -4063,6 +4074,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        $this->assertHasConnectionHandle();
 
                        $this->doRollback( $fname );
+                       $oldTrxShortId = $this->consumeTrxShortId();
                        $this->trxStatus = self::STATUS_TRX_NONE;
                        $this->trxAtomicLevels = [];
                        // Estimate the RTT via a query now that trxStatus is OK
@@ -4072,7 +4084,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                                $this->trxProfiler->transactionWritingOut(
                                        $this->server,
                                        $this->getDomainID(),
-                                       $this->trxShortId,
+                                       $oldTrxShortId,
                                        $writeTime,
                                        $this->trxWriteAffectedRows
                                );
@@ -4106,13 +4118,13 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         *
         * @see Database::rollback()
         * @param string $fname
+        * @throws DBError
         */
        protected function doRollback( $fname ) {
-               if ( $this->trxLevel ) {
+               if ( $this->trxLevel() ) {
                        # Disconnects cause rollback anyway, so ignore those errors
                        $ignoreErrors = true;
                        $this->query( 'ROLLBACK', $fname, $ignoreErrors );
-                       $this->trxLevel = 0;
                }
        }
 
@@ -4130,7 +4142,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        public function explicitTrxActive() {
-               return $this->trxLevel && ( $this->trxAtomicLevels || !$this->trxAutomatic );
+               return $this->trxLevel() && ( $this->trxAtomicLevels || !$this->trxAutomatic );
        }
 
        public function duplicateTableStructure(
@@ -4282,7 +4294,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         * @since 1.27
         */
        final protected function getRecordedTransactionLagStatus() {
-               return ( $this->trxLevel && $this->trxReplicaLag !== null )
+               return ( $this->trxLevel() && $this->trxReplicaLag !== null )
                        ? [ 'lag' => $this->trxReplicaLag, 'since' => $this->trxTimestamp() ]
                        : null;
        }
@@ -4830,7 +4842,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         * Run a few simple sanity checks and close dangling connections
         */
        public function __destruct() {
-               if ( $this->trxLevel && $this->trxDoneWrites ) {
+               if ( $this->trxLevel() && $this->trxDoneWrites ) {
                        trigger_error( "Uncommitted DB writes (transaction from {$this->trxFname})." );
                }
 
index 6c003dd..50aaff2 100644 (file)
@@ -28,6 +28,7 @@
 namespace Wikimedia\Rdbms;
 
 use Exception;
+use RuntimeException;
 use stdClass;
 use Wikimedia\AtEase\AtEase;
 
@@ -374,6 +375,17 @@ class DatabaseMssql extends Database {
                return $statementOnly;
        }
 
+       public function serverIsReadOnly() {
+               $encDatabase = $this->addQuotes( $this->getDBname() );
+               $res = $this->query(
+                       "SELECT IS_READ_ONLY FROM SYS.DATABASES WHERE NAME = $encDatabase",
+                       __METHOD__
+               );
+               $row = $this->fetchObject( $res );
+
+               return $row ? (bool)$row->IS_READ_ONLY : false;
+       }
+
        /**
         * @return int
         */
@@ -1071,13 +1083,10 @@ class DatabaseMssql extends Database {
                $this->query( 'ROLLBACK TRANSACTION ' . $this->addIdentifierQuotes( $identifier ), $fname );
        }
 
-       /**
-        * Begin a transaction, committing any previously open transaction
-        * @param string $fname
-        */
        protected function doBegin( $fname = __METHOD__ ) {
-               sqlsrv_begin_transaction( $this->conn );
-               $this->trxLevel = 1;
+               if ( !sqlsrv_begin_transaction( $this->conn ) ) {
+                       $this->reportQueryError( $this->lastError(), $this->lastErrno(), 'BEGIN', $fname );
+               }
        }
 
        /**
@@ -1085,8 +1094,9 @@ class DatabaseMssql extends Database {
         * @param string $fname
         */
        protected function doCommit( $fname = __METHOD__ ) {
-               sqlsrv_commit( $this->conn );
-               $this->trxLevel = 0;
+               if ( !sqlsrv_commit( $this->conn ) ) {
+                       $this->reportQueryError( $this->lastError(), $this->lastErrno(), 'COMMIT', $fname );
+               }
        }
 
        /**
@@ -1095,8 +1105,17 @@ class DatabaseMssql extends Database {
         * @param string $fname
         */
        protected function doRollback( $fname = __METHOD__ ) {
-               sqlsrv_rollback( $this->conn );
-               $this->trxLevel = 0;
+               if ( !sqlsrv_rollback( $this->conn ) ) {
+                       $this->queryLogger->error(
+                               "{fname}\t{db_server}\t{errno}\t{error}\t",
+                               $this->getLogContext( [
+                                       'errno' => $this->lastErrno(),
+                                       'error' => $this->lastError(),
+                                       'fname' => $fname,
+                                       'trace' => ( new RuntimeException() )->getTraceAsString()
+                               ] )
+                       );
+               }
        }
 
        /**
index a19a1a4..92eac90 100644 (file)
@@ -1072,7 +1072,7 @@ __INDEXATTR__;
         * @param string $desiredSchema
         */
        public function determineCoreSchema( $desiredSchema ) {
-               if ( $this->trxLevel ) {
+               if ( $this->trxLevel() ) {
                        // We do not want the schema selection to change on ROLLBACK or INSERT SELECT.
                        // See https://www.postgresql.org/docs/8.3/sql-set.html
                        throw new DBUnexpectedError(
index 46c34b4..17f12d3 100644 (file)
@@ -173,18 +173,8 @@ class DatabaseSqlite extends Database {
                        throw new DBExpectedError( $this, __CLASS__ . ": domain schemas are not supported." );
                }
 
-               $fileName = self::generateFileName( $this->dbDir, $dbName );
-               if ( !is_readable( $fileName ) ) {
-                       $error = "SQLite database file not readable";
-                       $this->connLogger->error(
-                               "Error connecting to {db_server}: {error}",
-                               $this->getLogContext( [ 'method' => __METHOD__, 'error' => $error ] )
-                       );
-                       throw new DBConnectionError( $this, $error );
-               }
-
                // Only $dbName is used, the other parameters are irrelevant for SQLite databases
-               $this->openFile( $fileName, $dbName, $tablePrefix );
+               $this->openFile( self::generateFileName( $this->dbDir, $dbName ), $dbName, $tablePrefix );
        }
 
        /**
@@ -196,6 +186,15 @@ class DatabaseSqlite extends Database {
         * @throws DBConnectionError
         */
        protected function openFile( $fileName, $dbName, $tablePrefix ) {
+               if ( !$this->hasMemoryPath() && !is_readable( $fileName ) ) {
+                       $error = "SQLite database file not readable";
+                       $this->connLogger->error(
+                               "Error connecting to {db_server}: {error}",
+                               $this->getLogContext( [ 'method' => __METHOD__, 'error' => $error ] )
+                       );
+                       throw new DBConnectionError( $this, $error );
+               }
+
                $this->dbPath = $fileName;
                try {
                        $this->conn = new PDO(
@@ -772,6 +771,17 @@ class DatabaseSqlite extends Database {
                return false;
        }
 
+       public function serverIsReadOnly() {
+               return ( !$this->hasMemoryPath() && !is_writable( $this->dbPath ) );
+       }
+
+       /**
+        * @return bool
+        */
+       private function hasMemoryPath() {
+               return ( strpos( $this->dbPath, ':memory:' ) === 0 );
+       }
+
        /**
         * @return string Wikitext of a link to the server software's web site
         */
@@ -815,7 +825,6 @@ class DatabaseSqlite extends Database {
                } else {
                        $this->query( 'BEGIN', $fname );
                }
-               $this->trxLevel = 1;
        }
 
        /**
index 3709de7..2ca3d7d 100644 (file)
@@ -35,7 +35,7 @@ class FakeResultWrapper extends ResultWrapper {
 
                $this->next();
 
-               return is_object( $row ) ? (array)$row : $row;
+               return is_object( $row ) ? get_object_vars( $row ) : $row;
        }
 
        function seek( $pos ) {
index c5dbfc5..35c9539 100644 (file)
@@ -140,7 +140,7 @@ interface ILBFactory {
        /**
         * Get cached (tracked) load balancers for all main database clusters
         *
-        * @return LoadBalancer[] Map of (cluster name => LoadBalancer)
+        * @return ILoadBalancer[] Map of (cluster name => ILoadBalancer)
         * @since 1.29
         */
        public function getAllMainLBs();
@@ -148,7 +148,7 @@ interface ILBFactory {
        /**
         * Get cached (tracked) load balancers for all external database clusters
         *
-        * @return LoadBalancer[] Map of (cluster name => LoadBalancer)
+        * @return ILoadBalancer[] Map of (cluster name => ILoadBalancer)
         * @since 1.29
         */
        public function getAllExternalLBs();
index aec99f4..f675b58 100644 (file)
@@ -34,55 +34,42 @@ use InvalidArgumentException;
 class LBFactoryMulti extends LBFactory {
        /** @var array A map of database names to section names */
        private $sectionsByDB;
-
        /**
         * @var array A 2-d map. For each section, gives a map of server names to
         * load ratios
         */
        private $sectionLoads;
-
        /**
         * @var array[] Server info associative array
         * @note The host, hostName and load entries will be overridden
         */
        private $serverTemplate;
 
-       // Optional settings
-
        /** @var array A 3-d map giving server load ratios for each section and group */
        private $groupLoadsBySection = [];
-
        /** @var array A 3-d map giving server load ratios by DB name */
        private $groupLoadsByDB = [];
-
        /** @var array A map of hostname to IP address */
        private $hostsByName = [];
-
        /** @var array A map of external storage cluster name to server load map */
        private $externalLoads = [];
-
        /**
         * @var array A set of server info keys overriding serverTemplate for
         * external storage
         */
        private $externalTemplateOverrides;
-
        /**
         * @var array A 2-d map overriding serverTemplate and
         * externalTemplateOverrides on a server-by-server basis. Applies to both
         * core and external storage
         */
        private $templateOverridesByServer;
-
        /** @var array A 2-d map overriding the server info by section */
        private $templateOverridesBySection;
-
        /** @var array A 2-d map overriding the server info by external storage cluster */
        private $templateOverridesByCluster;
-
        /** @var array An override array for all master servers */
        private $masterTemplateOverrides;
-
        /**
         * @var array|bool A map of section name to read-only message. Missing or
         * false for read/write
@@ -91,16 +78,12 @@ class LBFactoryMulti extends LBFactory {
 
        /** @var LoadBalancer[] */
        private $mainLBs = [];
-
        /** @var LoadBalancer[] */
        private $extLBs = [];
-
        /** @var string */
        private $loadMonitorClass = 'LoadMonitor';
-
        /** @var string */
        private $lastDomain;
-
        /** @var string */
        private $lastSection;
 
@@ -191,22 +174,19 @@ class LBFactoryMulti extends LBFactory {
                if ( $this->lastDomain === $domain ) {
                        return $this->lastSection;
                }
-               list( $dbName, ) = $this->getDBNameAndPrefix( $domain );
-               $section = $this->sectionsByDB[$dbName] ?? 'DEFAULT';
+
+               $database = $this->getDatabaseFromDomain( $domain );
+               $section = $this->sectionsByDB[$database] ?? 'DEFAULT';
                $this->lastSection = $section;
                $this->lastDomain = $domain;
 
                return $section;
        }
 
-       /**
-        * @param bool|string $domain
-        * @return LoadBalancer
-        */
        public function newMainLB( $domain = false ) {
-               list( $dbName, ) = $this->getDBNameAndPrefix( $domain );
+               $database = $this->getDatabaseFromDomain( $domain );
                $section = $this->getSectionForDomain( $domain );
-               $groupLoads = $this->groupLoadsByDB[$dbName] ?? [];
+               $groupLoads = $this->groupLoadsByDB[$database] ?? [];
 
                if ( isset( $this->groupLoadsBySection[$section] ) ) {
                        $groupLoads = array_merge_recursive(
@@ -232,10 +212,6 @@ class LBFactoryMulti extends LBFactory {
                );
        }
 
-       /**
-        * @param DatabaseDomain|string|bool $domain Domain ID, or false for the current domain
-        * @return LoadBalancer
-        */
        public function getMainLB( $domain = false ) {
                $section = $this->getSectionForDomain( $domain );
                if ( !isset( $this->mainLBs[$section] ) ) {
@@ -379,23 +355,14 @@ class LBFactoryMulti extends LBFactory {
 
        /**
         * @param DatabaseDomain|string|bool $domain Domain ID, or false for the current domain
-        * @return array [database name, table prefix]
+        * @return string
         */
-       private function getDBNameAndPrefix( $domain = false ) {
-               $domain = ( $domain === false )
-                       ? $this->localDomain
-                       : DatabaseDomain::newFromId( $domain );
-
-               return [ $domain->getDatabase(), $domain->getTablePrefix() ];
+       private function getDatabaseFromDomain( $domain = false ) {
+               return ( $domain === false )
+                       ? $this->localDomain->getDatabase()
+                       : DatabaseDomain::newFromId( $domain )->getDatabase();
        }
 
-       /**
-        * Execute a function for each tracked load balancer
-        * The callback is called with the load balancer as the first parameter,
-        * and $params passed as the subsequent parameters.
-        * @param callable $callback
-        * @param array $params
-        */
        public function forEachLB( $callback, array $params = [] ) {
                foreach ( $this->mainLBs as $lb ) {
                        $callback( $lb, ...$params );
index 49054e0..fd76d88 100644 (file)
@@ -70,20 +70,12 @@ class LBFactorySimple extends LBFactory {
                $this->loadMonitorClass = $conf['loadMonitorClass'] ?? 'LoadMonitor';
        }
 
-       /**
-        * @param bool|string $domain
-        * @return LoadBalancer
-        */
        public function newMainLB( $domain = false ) {
                return $this->newLoadBalancer( $this->servers );
        }
 
-       /**
-        * @param bool|string $domain
-        * @return LoadBalancer
-        */
        public function getMainLB( $domain = false ) {
-               if ( !isset( $this->mainLB ) ) {
+               if ( !$this->mainLB ) {
                        $this->mainLB = $this->newMainLB( $domain );
                }
 
@@ -132,14 +124,6 @@ class LBFactorySimple extends LBFactory {
                return $lb;
        }
 
-       /**
-        * Execute a function for each tracked load balancer
-        * The callback is called with the load balancer as the first parameter,
-        * and $params passed as the subsequent parameters.
-        *
-        * @param callable $callback
-        * @param array $params
-        */
        public function forEachLB( $callback, array $params = [] ) {
                if ( isset( $this->mainLB ) ) {
                        $callback( $this->mainLB, ...$params );
index 4d148b4..b086beb 100644 (file)
@@ -314,22 +314,6 @@ interface ILoadBalancer {
         */
        public function getWriterIndex();
 
-       /**
-        * Returns true if the specified index is a valid server index
-        *
-        * @param int $i
-        * @return bool
-        */
-       public function haveIndex( $i );
-
-       /**
-        * Returns true if the specified index is valid and has non-zero load
-        *
-        * @param int $i
-        * @return bool
-        */
-       public function isNonZeroLoad( $i );
-
        /**
         * Get the number of servers defined in configuration
         *
index 7f12d14..44d526c 100644 (file)
@@ -1306,10 +1306,24 @@ class LoadBalancer implements ILoadBalancer {
                return 0;
        }
 
+       /**
+        * Returns true if the specified index is a valid server index
+        *
+        * @param int $i
+        * @return bool
+        * @deprecated Since 1.34
+        */
        public function haveIndex( $i ) {
                return array_key_exists( $i, $this->servers );
        }
 
+       /**
+        * Returns true if the specified index is valid and has non-zero load
+        *
+        * @param int $i
+        * @return bool
+        * @deprecated Since 1.34
+        */
        public function isNonZeroLoad( $i ) {
                return array_key_exists( $i, $this->servers ) && $this->genericLoads[$i] != 0;
        }
index 173d741..c2fa687 100644 (file)
@@ -45,6 +45,7 @@ if ( !defined( 'MEDIAWIKI' ) ) {
 class CheckStorage {
        const CONCAT_HEADER = 'O:27:"concatenatedgziphistoryblob"';
        public $oldIdMap, $errors;
+       /** @var ExternalStoreDB */
        public $dbStore = null;
 
        public $errorDescriptions = [
@@ -223,7 +224,8 @@ class CheckStorage {
                        // Check external normal blobs for existence
                        if ( count( $externalNormalBlobs ) ) {
                                if ( is_null( $this->dbStore ) ) {
-                                       $this->dbStore = new ExternalStoreDB;
+                                       $esFactory = MediaWikiServices::getInstance()->getExternalStoreFactory();
+                                       $this->dbStore = $esFactory->getStore( 'DB' );
                                }
                                foreach ( $externalConcatBlobs as $cluster => $xBlobIds ) {
                                        $blobIds = array_keys( $xBlobIds );
@@ -422,7 +424,8 @@ class CheckStorage {
                }
 
                if ( is_null( $this->dbStore ) ) {
-                       $this->dbStore = new ExternalStoreDB;
+                       $esFactory = MediaWikiServices::getInstance()->getExternalStoreFactory();
+                       $this->dbStore = $esFactory->getStore( 'DB' );
                }
 
                foreach ( $externalConcatBlobs as $cluster => $oldIds ) {
index d3e9ce2..beb1975 100644 (file)
@@ -188,7 +188,9 @@ class CompressOld extends Maintenance {
 
                # Store in external storage if required
                if ( $extdb !== '' ) {
-                       $storeObj = new ExternalStoreDB;
+                       $esFactory = MediaWikiServices::getInstance()->getExternalStoreFactory();
+                       /** @var ExternalStoreDB $storeObj */
+                       $storeObj = $esFactory->getStore( 'DB' );
                        $compress = $storeObj->store( $extdb, $compress );
                        if ( $compress === false ) {
                                $this->error( "Unable to store object" );
@@ -232,7 +234,9 @@ class CompressOld extends Maintenance {
 
                # Set up external storage
                if ( $extdb != '' ) {
-                       $storeObj = new ExternalStoreDB;
+                       $esFactory = MediaWikiServices::getInstance()->getExternalStoreFactory();
+                       /** @var ExternalStoreDB $storeObj */
+                       $storeObj = $esFactory->getStore( 'DB' );
                }
 
                # Get all articles by page_id
index 0b95ba5..9554797 100644 (file)
@@ -21,6 +21,8 @@
  * @ingroup Maintenance ExternalStorage
  */
 
+use MediaWiki\MediaWikiServices;
+
 define( 'REPORTING_INTERVAL', 1 );
 
 if ( !defined( 'MEDIAWIKI' ) ) {
@@ -30,21 +32,22 @@ if ( !defined( 'MEDIAWIKI' ) ) {
 
        $fname = 'moveToExternal';
 
-       if ( !isset( $args[0] ) ) {
-               print "Usage: php moveToExternal.php [-s <startid>] [-e <endid>] <cluster>\n";
+       if ( !isset( $args[1] ) ) {
+               print "Usage: php moveToExternal.php [-s <startid>] [-e <endid>] <type> <location>\n";
                exit;
        }
 
-       $cluster = $args[0];
+       $type = $args[0]; // e.g. "DB" or "mwstore"
+       $location = $args[1]; // e.g. "cluster12" or "global-swift"
        $dbw = wfGetDB( DB_MASTER );
 
        $maxID = $options['e'] ?? $dbw->selectField( 'text', 'MAX(old_id)', '', $fname );
        $minID = $options['s'] ?? 1;
 
-       moveToExternal( $cluster, $maxID, $minID );
+       moveToExternal( $type, $location, $maxID, $minID );
 }
 
-function moveToExternal( $cluster, $maxID, $minID = 1 ) {
+function moveToExternal( $type, $location, $maxID, $minID = 1 ) {
        $fname = 'moveToExternal';
        $dbw = wfGetDB( DB_MASTER );
        $dbr = wfGetDB( DB_REPLICA );
@@ -53,7 +56,9 @@ function moveToExternal( $cluster, $maxID, $minID = 1 ) {
        $blockSize = 1000;
        $numBlocks = ceil( $count / $blockSize );
        print "Moving text rows from $minID to $maxID to external storage\n";
-       $ext = new ExternalStoreDB;
+
+       $esFactory = MediaWikiServices::getInstance()->getExternalStoreFactory();
+       $extStore = $esFactory->getStore( $type );
        $numMoved = 0;
 
        for ( $block = 0; $block < $numBlocks; $block++ ) {
@@ -108,7 +113,7 @@ function moveToExternal( $cluster, $maxID, $minID = 1 ) {
                        # print "Storing "  . strlen( $text ) . " bytes to $url\n";
                        # print "old_id=$id\n";
 
-                       $url = $ext->store( $cluster, $text );
+                       $url = $extStore->store( $location, $text );
                        if ( !$url ) {
                                print "Error writing to external storage\n";
                                exit;
index f17b00c..e6733a1 100644 (file)
@@ -69,6 +69,7 @@ class RecompressTracked {
        public $replicaId = false;
        public $noCount = false;
        public $debugLog, $infoLog, $criticalLog;
+       /** @var ExternalStoreDB */
        public $store;
 
        private static $optionsWithArgs = [
@@ -109,7 +110,8 @@ class RecompressTracked {
                foreach ( $options as $name => $value ) {
                        $this->$name = $value;
                }
-               $this->store = new ExternalStoreDB;
+               $esFactory = MediaWikiServices::getInstance()->getExternalStoreFactory();
+               $this->store = $esFactory->getStore( 'DB' );
                if ( !$this->isChild ) {
                        $GLOBALS['wgDebugLogPrefix'] = "RCT M: ";
                } elseif ( $this->replicaId !== false ) {
index b53b58f..0121f37 100644 (file)
                                                                e.keyCode === OO.ui.Keys.UP ? -1 : 1, 'wrap' )
                                                );
                                        }
-                                       if ( $field.is( ':input' ) ) {
+                                       if ( $field.is( 'input' ) ) {
                                                $field.trigger( 'select' );
                                        }
                                        return false;
                        if ( this.getValueAsDate() === null ) {
                                this.setValue( this.formatter.getDefaultDate() );
                        }
-                       if ( $field.is( ':input' ) ) {
+                       if ( $field.is( 'input' ) ) {
                                $field.trigger( 'select' );
                        }
 
index 7b017ab..033e2fe 100644 (file)
@@ -1753,8 +1753,10 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase {
         */
        public function testNewMutableRevisionFromArray_legacyEncoding( array $array ) {
                $cache = new WANObjectCache( [ 'cache' => new HashBagOStuff() ] );
-               $lb = MediaWikiServices::getInstance()->getDBLoadBalancer();
-               $blobStore = new SqlBlobStore( $lb, $cache );
+               $services = MediaWikiServices::getInstance();
+               $lb = $services->getDBLoadBalancer();
+               $access = $services->getExternalStoreAccess();
+               $blobStore = new SqlBlobStore( $lb, $access, $cache );
                $blobStore->setLegacyEncoding( 'windows-1252', Language::factory( 'en' ) );
 
                $factory = $this->getMockBuilder( BlobStoreFactory::class )
index 4f06ee2..84c815d 100644 (file)
@@ -55,7 +55,7 @@ class RevisionStoreFactoryTest extends MediaWikiTestCase {
         * @covers \MediaWiki\Revision\RevisionStoreFactory::getRevisionStore
         */
        public function testGetRevisionStore(
-               $wikiId,
+               $dbDomain,
                $mcrMigrationStage = MIGRATION_OLD,
                $contentHandlerUseDb = true
        ) {
@@ -81,14 +81,14 @@ class RevisionStoreFactoryTest extends MediaWikiTestCase {
                        $contentHandlerUseDb
                );
 
-               $store = $factory->getRevisionStore( $wikiId );
+               $store = $factory->getRevisionStore( $dbDomain );
                $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 );
+               $this->assertSame( $dbDomain, $wrapper->dbDomain );
 
                // ensure all other required services are correctly set
                $this->assertSame( $cache, $wrapper->cache );
index a8c8581..0648bfc 100644 (file)
@@ -450,9 +450,12 @@ class RevisionStoreTest extends MediaWikiTestCase {
                }
 
                $cache = new WANObjectCache( [ 'cache' => new HashBagOStuff() ] );
-               $lb = MediaWikiServices::getInstance()->getDBLoadBalancer();
+               $services = MediaWikiServices::getInstance();
+               $lb = $services->getDBLoadBalancer();
+               $access = $services->getExternalStoreAccess();
+
+               $blobStore = new SqlBlobStore( $lb, $access, $cache );
 
-               $blobStore = new SqlBlobStore( $lb, $cache );
                $blobStore->setLegacyEncoding( $encoding, Language::factory( $locale ) );
 
                $store = $this->getRevisionStore( $lb, $blobStore, $cache );
@@ -480,9 +483,11 @@ class RevisionStoreTest extends MediaWikiTestCase {
                ];
 
                $cache = new WANObjectCache( [ 'cache' => new HashBagOStuff() ] );
-               $lb = MediaWikiServices::getInstance()->getDBLoadBalancer();
+               $services = MediaWikiServices::getInstance();
+               $lb = $services->getDBLoadBalancer();
+               $access = $services->getExternalStoreAccess();
 
-               $blobStore = new SqlBlobStore( $lb, $cache );
+               $blobStore = new SqlBlobStore( $lb, $access, $cache );
                $blobStore->setLegacyEncoding( 'windows-1252', Language::factory( 'en' ) );
 
                $store = $this->getRevisionStore( $lb, $blobStore, $cache );
index 98f2980..d62e4c7 100644 (file)
@@ -438,10 +438,11 @@ class RevisionTest extends MediaWikiTestCase {
                $lb = $this->getMockBuilder( LoadBalancer::class )
                        ->disableOriginalConstructor()
                        ->getMock();
-
+               $access = MediaWikiServices::getInstance()->getExternalStoreAccess();
                $cache = $this->getWANObjectCache();
 
-               $blobStore = new SqlBlobStore( $lb, $cache );
+               $blobStore = new SqlBlobStore( $lb, $access, $cache );
+
                return $blobStore;
        }
 
@@ -807,7 +808,7 @@ class RevisionTest extends MediaWikiTestCase {
        public function testGetRevisionText_external_noOldId() {
                $this->setService(
                        'ExternalStoreFactory',
-                       new ExternalStoreFactory( [ 'ForTesting' ] )
+                       new ExternalStoreFactory( [ 'ForTesting' ], [ 'ForTesting://cluster1' ], 'test-id' )
                );
                $this->assertSame(
                        'AAAABBAAA',
@@ -829,14 +830,15 @@ class RevisionTest extends MediaWikiTestCase {
 
                $this->setService(
                        'ExternalStoreFactory',
-                       new ExternalStoreFactory( [ 'ForTesting' ] )
+                       new ExternalStoreFactory( [ 'ForTesting' ], [ 'ForTesting://cluster1' ], 'test-id' )
                );
 
                $lb = $this->getMockBuilder( LoadBalancer::class )
                        ->disableOriginalConstructor()
                        ->getMock();
+               $access = MediaWikiServices::getInstance()->getExternalStoreAccess();
 
-               $blobStore = new SqlBlobStore( $lb, $cache );
+               $blobStore = new SqlBlobStore( $lb, $access, $cache );
                $this->setService( 'BlobStoreFactory', $this->mockBlobStoreFactory( $blobStore ) );
 
                $this->assertSame(
index 5506940..ac39b48 100644 (file)
@@ -24,6 +24,7 @@ class SqlBlobStoreTest extends MediaWikiTestCase {
 
                $store = new SqlBlobStore(
                        $services->getDBLoadBalancer(),
+                       $services->getExternalStoreAccess(),
                        $services->getMainWANObjectCache()
                );
 
diff --git a/tests/phpunit/includes/externalstore/ExternalStoreAccessTest.php b/tests/phpunit/includes/externalstore/ExternalStoreAccessTest.php
new file mode 100644 (file)
index 0000000..80e836f
--- /dev/null
@@ -0,0 +1,100 @@
+<?php
+
+use Wikimedia\Rdbms\LBFactory;
+
+/**
+ * @covers ExternalStoreAccess
+ */
+class ExternalStoreAccessTest extends MediaWikiTestCase {
+
+       use MediaWikiCoversValidator;
+
+       /**
+        * @covers ExternalStoreAccess::isReadOnly
+        */
+       public function testBasic() {
+               $active = [ 'memory' ];
+               $defaults = [ 'memory://cluster1', 'memory://cluster2' ];
+               $esFactory = new ExternalStoreFactory( $active, $defaults, 'db-prefix' );
+               $access = new ExternalStoreAccess( $esFactory );
+
+               $this->assertEquals( false, $access->isReadOnly() );
+
+               /** @var ExternalStoreMemory $store */
+               $store = $esFactory->getStore( 'memory' );
+               $this->assertInstanceOf( ExternalStoreMemory::class, $store );
+
+               $lb = $this->getMockBuilder( LoadBalancer::class )
+                       ->disableOriginalConstructor()->getMock();
+               $lb->expects( $this->any() )->method( 'getReadOnlyReason' )->willReturn( 'Locked' );
+               $lb->expects( $this->any() )->method( 'getServerInfo' )->willReturn( [] );
+
+               $lbFactory = $this->getMockBuilder( LBFactory::class )
+                       ->disableOriginalConstructor()->getMock();
+               $lbFactory->expects( $this->any() )->method( 'getExternalLB' )->willReturn( $lb );
+
+               $this->setService( 'DBLoadBalancerFactory', $lbFactory );
+
+               $active = [ 'db', 'mwstore' ];
+               $defaults = [ 'DB://clusterX' ];
+               $esFactory = new ExternalStoreFactory( $active, $defaults, 'db-prefix' );
+               $access = new ExternalStoreAccess( $esFactory );
+               $this->assertEquals( true, $access->isReadOnly() );
+
+               $store->clear();
+       }
+
+       /**
+        * @covers ExternalStoreAccess::fetchFromURL
+        * @covers ExternalStoreAccess::fetchFromURLs
+        * @covers ExternalStoreAccess::insert
+        */
+       public function testReadWrite() {
+               $active = [ 'memory' ]; // active store types
+               $defaults = [ 'memory://cluster1', 'memory://cluster2' ];
+               $esFactory = new ExternalStoreFactory( $active, $defaults, 'db-prefix' );
+               $access = new ExternalStoreAccess( $esFactory );
+
+               /** @var ExternalStoreMemory $storeLocal */
+               $storeLocal = $esFactory->getStore( 'memory' );
+               /** @var ExternalStoreMemory $storeOther */
+               $storeOther = $esFactory->getStore( 'memory', [ 'domain' => 'other' ] );
+               $this->assertInstanceOf( ExternalStoreMemory::class, $storeLocal );
+               $this->assertInstanceOf( ExternalStoreMemory::class, $storeOther );
+
+               $v1 = wfRandomString();
+               $v2 = wfRandomString();
+               $v3 = wfRandomString();
+
+               $this->assertEquals( false, $storeLocal->fetchFromURL( 'memory://cluster1/1' ) );
+
+               $url1 = 'memory://cluster1/1';
+               $this->assertEquals(
+                       $url1,
+                       $esFactory->getStoreForUrl( 'memory://cluster1' )
+                               ->store( $esFactory->getStoreLocationFromUrl( 'memory://cluster1' ), $v1 )
+               );
+               $this->assertEquals(
+                       $v1,
+                       $esFactory->getStoreForUrl( 'memory://cluster1/1' )
+                               ->fetchFromURL( 'memory://cluster1/1' )
+               );
+               $this->assertEquals( $v1, $storeLocal->fetchFromURL( 'memory://cluster1/1' ) );
+
+               $url2 = $access->insert( $v2 );
+               $url3 = $access->insert( $v3, [ 'domain' => 'other' ] );
+               $this->assertNotFalse( $url2 );
+               $this->assertNotFalse( $url3 );
+               // There is only one active store type
+               $this->assertEquals( $v2, $storeLocal->fetchFromURL( $url2 ) );
+               $this->assertEquals( $v3, $storeOther->fetchFromURL( $url3 ) );
+               $this->assertEquals( false, $storeOther->fetchFromURL( $url2 ) );
+               $this->assertEquals( false, $storeLocal->fetchFromURL( $url3 ) );
+
+               $res = $access->fetchFromURLs( [ $url1, $url2, $url3 ] );
+               $this->assertEquals( [ $url1 => $v1, $url2 => $v2, $url3 => false ], $res, "Local-only" );
+
+               $storeLocal->clear();
+               $storeOther->clear();
+       }
+}
index f762693..e63ce59 100644 (file)
@@ -2,15 +2,26 @@
 
 /**
  * @covers ExternalStoreFactory
+ * @covers ExternalStoreAccess
  */
-class ExternalStoreFactoryTest extends PHPUnit\Framework\TestCase {
+class ExternalStoreFactoryTest extends MediaWikiTestCase {
 
        use MediaWikiCoversValidator;
 
-       public function testExternalStoreFactory_noStores() {
-               $factory = new ExternalStoreFactory( [] );
-               $this->assertFalse( $factory->getStoreObject( 'ForTesting' ) );
-               $this->assertFalse( $factory->getStoreObject( 'foo' ) );
+       /**
+        * @expectedException ExternalStoreException
+        */
+       public function testExternalStoreFactory_noStores1() {
+               $factory = new ExternalStoreFactory( [], [], 'test-id' );
+               $factory->getStore( 'ForTesting' );
+       }
+
+       /**
+        * @expectedException ExternalStoreException
+        */
+       public function testExternalStoreFactory_noStores2() {
+               $factory = new ExternalStoreFactory( [], [], 'test-id' );
+               $factory->getStore( 'foo' );
        }
 
        public function provideStoreNames() {
@@ -24,18 +35,108 @@ class ExternalStoreFactoryTest extends PHPUnit\Framework\TestCase {
         * @dataProvider provideStoreNames
         */
        public function testExternalStoreFactory_someStore_protoMatch( $proto ) {
-               $factory = new ExternalStoreFactory( [ 'ForTesting' ] );
-               $store = $factory->getStoreObject( $proto );
+               $factory = new ExternalStoreFactory( [ 'ForTesting' ], [], 'test-id' );
+               $store = $factory->getStore( $proto );
                $this->assertInstanceOf( ExternalStoreForTesting::class, $store );
        }
 
        /**
         * @dataProvider provideStoreNames
+        * @expectedException ExternalStoreException
         */
        public function testExternalStoreFactory_someStore_noProtoMatch( $proto ) {
-               $factory = new ExternalStoreFactory( [ 'SomeOtherClassName' ] );
-               $store = $factory->getStoreObject( $proto );
-               $this->assertFalse( $store );
+               $factory = new ExternalStoreFactory( [ 'SomeOtherClassName' ], [], 'test-id' );
+               $factory->getStore( $proto );
+       }
+
+       /**
+        * @covers ExternalStoreFactory::getProtocols
+        * @covers ExternalStoreFactory::getWriteBaseUrls
+        * @covers ExternalStoreFactory::getStore
+        */
+       public function testStoreFactoryBasic() {
+               $active = [ 'memory' ];
+               $defaults = [ 'memory://cluster1', 'memory://cluster2' ];
+               $esFactory = new ExternalStoreFactory( $active, $defaults, 'db-prefix' );
+
+               $this->assertEquals( $active, $esFactory->getProtocols() );
+               $this->assertEquals( $defaults, $esFactory->getWriteBaseUrls() );
+
+               /** @var ExternalStoreMemory $store */
+               $store = $esFactory->getStore( 'memory' );
+               $this->assertInstanceOf( ExternalStoreMemory::class, $store );
+               $this->assertEquals( false, $store->isReadOnly( 'cluster1' ) );
+               $this->assertEquals( false, $store->isReadOnly( 'cluster2' ) );
+               $this->assertEquals( true, $store->isReadOnly( 'clusterOld' ) );
+
+               $lb = $this->getMockBuilder( \Wikimedia\Rdbms\LoadBalancer::class )
+                       ->disableOriginalConstructor()->getMock();
+               $lb->expects( $this->any() )->method( 'getReadOnlyReason' )->willReturn( 'Locked' );
+               $lbFactory = $this->getMockBuilder( \Wikimedia\Rdbms\LBFactory::class )
+                       ->disableOriginalConstructor()->getMock();
+               $lbFactory->expects( $this->any() )->method( 'getExternalLB' )->willReturn( $lb );
+
+               $this->setService( 'DBLoadBalancerFactory', $lbFactory );
+
+               $active = [ 'db', 'mwstore' ];
+               $defaults = [ 'db://clusterX' ];
+               $esFactory = new ExternalStoreFactory( $active, $defaults, 'db-prefix' );
+               $this->assertEquals( $active, $esFactory->getProtocols() );
+               $this->assertEquals( $defaults, $esFactory->getWriteBaseUrls() );
+
+               $store->clear();
        }
 
+       /**
+        * @covers ExternalStoreFactory::getStoreForUrl
+        * @covers ExternalStoreFactory::getStoreLocationFromUrl
+        */
+       public function testStoreFactoryReadWrite() {
+               $active = [ 'memory' ]; // active store types
+               $defaults = [ 'memory://cluster1', 'memory://cluster2' ];
+               $esFactory = new ExternalStoreFactory( $active, $defaults, 'db-prefix' );
+               $access = new ExternalStoreAccess( $esFactory );
+
+               /** @var ExternalStoreMemory $storeLocal */
+               $storeLocal = $esFactory->getStore( 'memory' );
+               /** @var ExternalStoreMemory $storeOther */
+               $storeOther = $esFactory->getStore( 'memory', [ 'domain' => 'other' ] );
+               $this->assertInstanceOf( ExternalStoreMemory::class, $storeLocal );
+               $this->assertInstanceOf( ExternalStoreMemory::class, $storeOther );
+
+               $v1 = wfRandomString();
+               $v2 = wfRandomString();
+               $v3 = wfRandomString();
+
+               $this->assertEquals( false, $storeLocal->fetchFromURL( 'memory://cluster1/1' ) );
+
+               $url1 = 'memory://cluster1/1';
+               $this->assertEquals(
+                       $url1,
+                       $esFactory->getStoreForUrl( 'memory://cluster1' )
+                               ->store( $esFactory->getStoreLocationFromUrl( 'memory://cluster1' ), $v1 )
+               );
+               $this->assertEquals(
+                       $v1,
+                       $esFactory->getStoreForUrl( 'memory://cluster1/1' )
+                               ->fetchFromURL( 'memory://cluster1/1' )
+               );
+               $this->assertEquals( $v1, $storeLocal->fetchFromURL( 'memory://cluster1/1' ) );
+
+               $url2 = $access->insert( $v2 );
+               $url3 = $access->insert( $v3, [ 'domain' => 'other' ] );
+               $this->assertNotFalse( $url2 );
+               $this->assertNotFalse( $url3 );
+               // There is only one active store type
+               $this->assertEquals( $v2, $storeLocal->fetchFromURL( $url2 ) );
+               $this->assertEquals( $v3, $storeOther->fetchFromURL( $url3 ) );
+               $this->assertEquals( false, $storeOther->fetchFromURL( $url2 ) );
+               $this->assertEquals( false, $storeLocal->fetchFromURL( $url3 ) );
+
+               $res = $access->fetchFromURLs( [ $url1, $url2, $url3 ] );
+               $this->assertEquals( [ $url1 => $v1, $url2 => $v2, $url3 => false ], $res, "Local-only" );
+
+               $storeLocal->clear();
+               $storeOther->clear();
+       }
 }
index 7ca3874..60db27d 100644 (file)
@@ -8,7 +8,7 @@ class ExternalStoreTest extends MediaWikiTestCase {
        public function testExternalFetchFromURL_noExternalStores() {
                $this->setService(
                        'ExternalStoreFactory',
-                       new ExternalStoreFactory( [] )
+                       new ExternalStoreFactory( [], [], 'test-id' )
                );
 
                $this->assertFalse(
@@ -23,7 +23,7 @@ class ExternalStoreTest extends MediaWikiTestCase {
        public function testExternalFetchFromURL_someExternalStore() {
                $this->setService(
                        'ExternalStoreFactory',
-                       new ExternalStoreFactory( [ 'ForTesting' ] )
+                       new ExternalStoreFactory( [ 'ForTesting' ], [ 'ForTesting://cluster1' ], 'test-id' )
                );
 
                $this->assertEquals(
index 812702b..34ddb1f 100644 (file)
@@ -877,7 +877,7 @@ EOF
 
                $bClocks = $b->mParseStartTime;
 
-               $a->mergeInternalMetaDataFrom( $b->object, 'b' );
+               $a->mergeInternalMetaDataFrom( $b->object );
                $mergedClocks = $a->mParseStartTime;
 
                foreach ( $mergedClocks as $clock => $timestamp ) {
@@ -890,7 +890,7 @@ EOF
                $a->resetParseStartTime();
                $aClocks = $a->mParseStartTime;
 
-               $a->mergeInternalMetaDataFrom( $b->object, 'b' );
+               $a->mergeInternalMetaDataFrom( $b->object );
                $mergedClocks = $a->mParseStartTime;
 
                foreach ( $mergedClocks as $clock => $timestamp ) {
@@ -902,7 +902,7 @@ EOF
                $a = new ParserOutput();
                $a = TestingAccessWrapper::newFromObject( $a );
 
-               $a->mergeInternalMetaDataFrom( $b->object, 'b' );
+               $a->mergeInternalMetaDataFrom( $b->object );
                $mergedClocks = $a->mParseStartTime;
 
                foreach ( $mergedClocks as $clock => $timestamp ) {