From baafb5adb400098f475c20fe10e3a181e87a4795 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Mon, 26 Feb 2018 22:24:46 -0800 Subject: [PATCH] Make ExternalStore wrap ExternalStoreFactory and create access class * 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 --- autoload.php | 3 + includes/MediaWikiServices.php | 8 + includes/Revision/RevisionRenderer.php | 1 - includes/Revision/RevisionStore.php | 1 - includes/ServiceWiring.php | 14 +- includes/Storage/BlobStoreFactory.php | 9 + includes/Storage/SqlBlobStore.php | 24 ++- includes/externalstore/ExternalStore.php | 170 +++++------------- .../externalstore/ExternalStoreAccess.php | 164 +++++++++++++++++ includes/externalstore/ExternalStoreDB.php | 83 +++++---- .../externalstore/ExternalStoreException.php | 5 + .../externalstore/ExternalStoreFactory.php | 168 +++++++++++++++-- .../externalstore/ExternalStoreMedium.php | 52 ++++-- .../externalstore/ExternalStoreMemory.php | 102 +++++++++++ .../externalstore/ExternalStoreMwstore.php | 46 +++-- includes/historyblob/HistoryBlobStub.php | 7 +- maintenance/storage/checkStorage.php | 7 +- maintenance/storage/compressOld.php | 8 +- maintenance/storage/moveToExternal.php | 19 +- maintenance/storage/recompressTracked.php | 4 +- .../Revision/RevisionStoreDbTestBase.php | 6 +- .../includes/Revision/RevisionStoreTest.php | 13 +- tests/phpunit/includes/RevisionTest.php | 12 +- .../includes/Storage/SqlBlobStoreTest.php | 1 + .../externalstore/ExternalStoreAccessTest.php | 100 +++++++++++ .../ExternalStoreFactoryTest.php | 121 +++++++++++-- .../externalstore/ExternalStoreTest.php | 4 +- 27 files changed, 911 insertions(+), 241 deletions(-) create mode 100644 includes/externalstore/ExternalStoreAccess.php create mode 100644 includes/externalstore/ExternalStoreException.php create mode 100644 includes/externalstore/ExternalStoreMemory.php create mode 100644 tests/phpunit/includes/externalstore/ExternalStoreAccessTest.php diff --git a/autoload.php b/autoload.php index b01827aa3f..6457747c84 100644 --- a/autoload.php +++ b/autoload.php @@ -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', diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php index 689477b545..a37e32e675 100644 --- a/includes/MediaWikiServices.php +++ b/includes/MediaWikiServices.php @@ -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 diff --git a/includes/Revision/RevisionRenderer.php b/includes/Revision/RevisionRenderer.php index 80aee783a3..99150c1368 100644 --- a/includes/Revision/RevisionRenderer.php +++ b/includes/Revision/RevisionRenderer.php @@ -69,7 +69,6 @@ class RevisionRenderer { $this->loadBalancer = $loadBalancer; $this->roleRegistery = $roleRegistry; $this->dbDomain = $dbDomain; - $this->saveParseLogger = new NullLogger(); } diff --git a/includes/Revision/RevisionStore.php b/includes/Revision/RevisionStore.php index 0bfb39326c..f269afe639 100644 --- a/includes/Revision/RevisionStore.php +++ b/includes/Revision/RevisionStore.php @@ -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, diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index e371b5a5c8..4712a0a8b2 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -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' ) ); }, diff --git a/includes/Storage/BlobStoreFactory.php b/includes/Storage/BlobStoreFactory.php index 82624467ae..b59c68d175 100644 --- a/includes/Storage/BlobStoreFactory.php +++ b/includes/Storage/BlobStoreFactory.php @@ -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 ); diff --git a/includes/Storage/SqlBlobStore.php b/includes/Storage/SqlBlobStore.php index 7fe56438f8..5260754f5f 100644 --- a/includes/Storage/SqlBlobStore.php +++ b/includes/Storage/SqlBlobStore.php @@ -27,13 +27,13 @@ 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; } diff --git a/includes/externalstore/ExternalStore.php b/includes/externalstore/ExternalStore.php index 76f20f0ed3..7c90b35075 100644 --- a/includes/externalstore/ExternalStore.php +++ b/includes/externalstore/ExternalStore.php @@ -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 index 0000000000..8603cc2697 --- /dev/null +++ b/includes/externalstore/ExternalStoreAccess.php @@ -0,0 +1,164 @@ +:///". 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 + } +} diff --git a/includes/externalstore/ExternalStoreDB.php b/includes/externalstore/ExternalStoreDB.php index 15bc3e0eb7..feb0614d1e 100644 --- a/includes/externalstore/ExternalStoreDB.php +++ b/includes/externalstore/ExternalStoreDB.php @@ -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 index 0000000000..a2ef27de72 --- /dev/null +++ b/includes/externalstore/ExternalStoreException.php @@ -0,0 +1,5 @@ +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) ":///", for retrieval, or + * - b) "://", 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; + } } diff --git a/includes/externalstore/ExternalStoreMedium.php b/includes/externalstore/ExternalStoreMedium.php index da7752b745..0cdcf53ee7 100644 --- a/includes/externalstore/ExternalStoreMedium.php +++ b/includes/externalstore/ExternalStoreMedium.php @@ -21,22 +21,55 @@ * @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 ":///". 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 index 0000000000..daad75c485 --- /dev/null +++ b/includes/externalstore/ExternalStoreMemory.php @@ -0,0 +1,102 @@ + 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 ]; + } +} diff --git a/includes/externalstore/ExternalStoreMwstore.php b/includes/externalstore/ExternalStoreMwstore.php index 7414f23e9e..77c23ee395 100644 --- a/includes/externalstore/ExternalStoreMwstore.php +++ b/includes/externalstore/ExternalStoreMwstore.php @@ -31,6 +31,22 @@ * @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; } diff --git a/includes/historyblob/HistoryBlobStub.php b/includes/historyblob/HistoryBlobStub.php index 4995d3b3f0..9a4df1f81d 100644 --- a/includes/historyblob/HistoryBlobStub.php +++ b/includes/historyblob/HistoryBlobStub.php @@ -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 ) ) { diff --git a/maintenance/storage/checkStorage.php b/maintenance/storage/checkStorage.php index 173d741be8..c2fa687432 100644 --- a/maintenance/storage/checkStorage.php +++ b/maintenance/storage/checkStorage.php @@ -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 ) { diff --git a/maintenance/storage/compressOld.php b/maintenance/storage/compressOld.php index d3e9ce2cf6..beb1975fad 100644 --- a/maintenance/storage/compressOld.php +++ b/maintenance/storage/compressOld.php @@ -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 diff --git a/maintenance/storage/moveToExternal.php b/maintenance/storage/moveToExternal.php index 0b95ba5e68..9554797f44 100644 --- a/maintenance/storage/moveToExternal.php +++ b/maintenance/storage/moveToExternal.php @@ -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 ] [-e ] \n"; + if ( !isset( $args[1] ) ) { + print "Usage: php moveToExternal.php [-s ] [-e ] \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; diff --git a/maintenance/storage/recompressTracked.php b/maintenance/storage/recompressTracked.php index f17b00c1c8..e6733a184b 100644 --- a/maintenance/storage/recompressTracked.php +++ b/maintenance/storage/recompressTracked.php @@ -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 ) { diff --git a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php index 7b017ab67f..033e2feb55 100644 --- a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php +++ b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php @@ -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 ) diff --git a/tests/phpunit/includes/Revision/RevisionStoreTest.php b/tests/phpunit/includes/Revision/RevisionStoreTest.php index a8c8581106..0648bfce6e 100644 --- a/tests/phpunit/includes/Revision/RevisionStoreTest.php +++ b/tests/phpunit/includes/Revision/RevisionStoreTest.php @@ -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 ); diff --git a/tests/phpunit/includes/RevisionTest.php b/tests/phpunit/includes/RevisionTest.php index 98f2980f93..d62e4c7402 100644 --- a/tests/phpunit/includes/RevisionTest.php +++ b/tests/phpunit/includes/RevisionTest.php @@ -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( diff --git a/tests/phpunit/includes/Storage/SqlBlobStoreTest.php b/tests/phpunit/includes/Storage/SqlBlobStoreTest.php index 55069403d6..ac39b48cb9 100644 --- a/tests/phpunit/includes/Storage/SqlBlobStoreTest.php +++ b/tests/phpunit/includes/Storage/SqlBlobStoreTest.php @@ -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 index 0000000000..80e836fa58 --- /dev/null +++ b/tests/phpunit/includes/externalstore/ExternalStoreAccessTest.php @@ -0,0 +1,100 @@ +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(); + } +} diff --git a/tests/phpunit/includes/externalstore/ExternalStoreFactoryTest.php b/tests/phpunit/includes/externalstore/ExternalStoreFactoryTest.php index f762693864..e63ce59493 100644 --- a/tests/phpunit/includes/externalstore/ExternalStoreFactoryTest.php +++ b/tests/phpunit/includes/externalstore/ExternalStoreFactoryTest.php @@ -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(); + } } diff --git a/tests/phpunit/includes/externalstore/ExternalStoreTest.php b/tests/phpunit/includes/externalstore/ExternalStoreTest.php index 7ca38749fa..60db27da37 100644 --- a/tests/phpunit/includes/externalstore/ExternalStoreTest.php +++ b/tests/phpunit/includes/externalstore/ExternalStoreTest.php @@ -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( -- 2.20.1