Make ExternalStore wrap ExternalStoreFactory and create access class
authorAaron Schulz <aschulz@wikimedia.org>
Tue, 27 Feb 2018 06:24:46 +0000 (22:24 -0800)
committerAaron Schulz <aschulz@wikimedia.org>
Fri, 28 Jun 2019 21:31:44 +0000 (14:31 -0700)
* Inject settings and global instances as dependencies to the
  ExternalStoreMedium instances. This includes the local wiki
  domain, so that wfWikiId() calls are not scattered around.
* Create ExternalStoreAccess service for read/write logic.
* Deprecate the ExternalStore wrapper methods.
* Add some exception cases for bogus store URLs are used instead
  of just giving PHP warnings and failing later.
* Make moveToExternal.php require the type/protocol to decide
  which ExternalStoreMedium to use instead of assuming "DB".
* Convert logging calls to use LoggerInterface.

Change-Id: I40c3b5534fc8a31116c4c5eb64ee6e4903a6197a

27 files changed:
autoload.php
includes/MediaWikiServices.php
includes/Revision/RevisionRenderer.php
includes/Revision/RevisionStore.php
includes/ServiceWiring.php
includes/Storage/BlobStoreFactory.php
includes/Storage/SqlBlobStore.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
maintenance/storage/checkStorage.php
maintenance/storage/compressOld.php
maintenance/storage/moveToExternal.php
maintenance/storage/recompressTracked.php
tests/phpunit/includes/Revision/RevisionStoreDbTestBase.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

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 80aee78..99150c1 100644 (file)
@@ -69,7 +69,6 @@ class RevisionRenderer {
                $this->loadBalancer = $loadBalancer;
                $this->roleRegistery = $roleRegistry;
                $this->dbDomain = $dbDomain;
-
                $this->saveParseLogger = new NullLogger();
        }
 
index 0bfb393..f269afe 100644 (file)
@@ -154,7 +154,6 @@ class RevisionStore
         * @param int $mcrMigrationStage An appropriate combination of SCHEMA_COMPAT_XXX flags
         * @param ActorMigration $actorMigration
         * @param bool|string $dbDomain DB domain of the relevant wiki or false for the current one
-        *
         */
        public function __construct(
                ILoadBalancer $loadBalancer,
index e371b5a..4712a0a 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 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 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 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 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(