From: addshore Date: Tue, 14 Nov 2017 11:17:34 +0000 (+0000) Subject: Introduce ExternalStoreFactory X-Git-Tag: 1.31.0-rc.0~1528^2 X-Git-Url: http://git.cyclocoop.org/%22%20.%20generer_url_ecrire%28%22statistiques_visites%22%2C%22%22%29%20.%20%22?a=commitdiff_plain;h=24b24e493ee736f47c71a9bb223803fce9385585;p=lhc%2Fweb%2Fwiklou.git Introduce ExternalStoreFactory Change-Id: If0d8f503e3cc9fd83f3b40e2ac8a5f9dc8b7e0ea --- diff --git a/autoload.php b/autoload.php index bfc892878c..89c4473035 100644 --- a/autoload.php +++ b/autoload.php @@ -454,6 +454,7 @@ $wgAutoloadLocalClasses = [ 'ExtensionRegistry' => __DIR__ . '/includes/registration/ExtensionRegistry.php', 'ExternalStore' => __DIR__ . '/includes/externalstore/ExternalStore.php', 'ExternalStoreDB' => __DIR__ . '/includes/externalstore/ExternalStoreDB.php', + 'ExternalStoreFactory' => __DIR__ . '/includes/externalstore/ExternalStoreFactory.php', 'ExternalStoreHttp' => __DIR__ . '/includes/externalstore/ExternalStoreHttp.php', 'ExternalStoreMedium' => __DIR__ . '/includes/externalstore/ExternalStoreMedium.php', 'ExternalStoreMwstore' => __DIR__ . '/includes/externalstore/ExternalStoreMwstore.php', diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php index 0d010b49d1..b39c8a4d6b 100644 --- a/includes/MediaWikiServices.php +++ b/includes/MediaWikiServices.php @@ -690,6 +690,14 @@ class MediaWikiServices extends ServiceContainer { return $this->getService( 'ShellCommandFactory' ); } + /** + * @since 1.31 + * @return \ExternalStoreFactory + */ + public function getExternalStoreFactory() { + return $this->getService( 'ExternalStoreFactory' ); + } + /////////////////////////////////////////////////////////////////////////// // NOTE: When adding a service getter here, don't forget to add a test // case for it in MediaWikiServicesTest::provideGetters() and in diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index 0496b67fc7..ae88d375cc 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -447,6 +447,14 @@ return [ return $factory; }, + 'ExternalStoreFactory' => function ( MediaWikiServices $services ) { + $config = $services->getMainConfig(); + + return new ExternalStoreFactory( + $config->get( 'ExternalStores' ) + ); + }, + /////////////////////////////////////////////////////////////////////////// // NOTE: When adding a service here, don't forget to add a getter function // in the MediaWikiServices class. The convenience getter should just call diff --git a/includes/externalstore/ExternalStore.php b/includes/externalstore/ExternalStore.php index 1563baf8af..3beab29532 100644 --- a/includes/externalstore/ExternalStore.php +++ b/includes/externalstore/ExternalStore.php @@ -3,6 +3,8 @@ * @defgroup ExternalStorage ExternalStorage */ +use MediaWiki\MediaWikiServices; + /** * Interface for data storage in external repositories. * @@ -52,16 +54,9 @@ class ExternalStore { * @return ExternalStoreMedium|bool The store class or false on error */ public static function getStoreObject( $proto, array $params = [] ) { - global $wgExternalStores; - - if ( !$wgExternalStores || !in_array( $proto, $wgExternalStores ) ) { - return false; // protocol not enabled - } - - $class = 'ExternalStore' . ucfirst( $proto ); - - // Any custom modules should be added to $wgAutoLoadClasses for on-demand loading - return class_exists( $class ) ? new $class( $params ) : false; + return MediaWikiServices::getInstance() + ->getExternalStoreFactory() + ->getStoreObject( $proto, $params ); } /** diff --git a/includes/externalstore/ExternalStoreFactory.php b/includes/externalstore/ExternalStoreFactory.php new file mode 100644 index 0000000000..940fb2e20c --- /dev/null +++ b/includes/externalstore/ExternalStoreFactory.php @@ -0,0 +1,42 @@ +externalStores = array_map( 'strtolower', $externalStores ); + } + + /** + * Get an external store object of the given type, with the given parameters + * + * @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 + */ + public function getStoreObject( $proto, array $params = [] ) { + if ( !$this->externalStores || !in_array( strtolower( $proto ), $this->externalStores ) ) { + // Protocol not enabled + return false; + } + + $class = 'ExternalStore' . ucfirst( $proto ); + + // Any custom modules should be added to $wgAutoLoadClasses for on-demand loading + return class_exists( $class ) ? new $class( $params ) : false; + } + +} diff --git a/tests/common/TestsAutoLoader.php b/tests/common/TestsAutoLoader.php index f7bf7a6f57..993f8d3950 100644 --- a/tests/common/TestsAutoLoader.php +++ b/tests/common/TestsAutoLoader.php @@ -105,6 +105,9 @@ $wgAutoloadClasses += [ # tests/phpunit/includes/diff 'FakeDiffOp' => "$testDir/phpunit/includes/diff/FakeDiffOp.php", + # tests/phpunit/includes/externalstore + 'ExternalStoreForTesting' => "$testDir/phpunit/includes/externalstore/ExternalStoreForTesting.php", + # tests/phpunit/includes/logging 'LogFormatterTestCase' => "$testDir/phpunit/includes/logging/LogFormatterTestCase.php", diff --git a/tests/phpunit/includes/externalstore/ExternalStoreFactoryTest.php b/tests/phpunit/includes/externalstore/ExternalStoreFactoryTest.php new file mode 100644 index 0000000000..a0bac63958 --- /dev/null +++ b/tests/phpunit/includes/externalstore/ExternalStoreFactoryTest.php @@ -0,0 +1,39 @@ +assertFalse( $factory->getStoreObject( 'ForTesting' ) ); + $this->assertFalse( $factory->getStoreObject( 'foo' ) ); + } + + public function provideStoreNames() { + yield 'Same case as construction' => [ 'ForTesting' ]; + yield 'All lower case' => [ 'fortesting' ]; + yield 'All upper case' => [ 'FORTESTING' ]; + yield 'Mix of cases' => [ 'FOrTEsTInG' ]; + } + + /** + * @dataProvider provideStoreNames + */ + public function testExternalStoreFactory_someStore_protoMatch( $proto ) { + $factory = new ExternalStoreFactory( [ 'ForTesting' ] ); + $store = $factory->getStoreObject( $proto ); + $this->assertInstanceOf( ExternalStoreForTesting::class, $store ); + } + + /** + * @dataProvider provideStoreNames + */ + public function testExternalStoreFactory_someStore_noProtoMatch( $proto ) { + $factory = new ExternalStoreFactory( [ 'SomeOtherClassName' ] ); + $store = $factory->getStoreObject( $proto ); + $this->assertFalse( $store ); + } + +} diff --git a/tests/phpunit/includes/externalstore/ExternalStoreForTesting.php b/tests/phpunit/includes/externalstore/ExternalStoreForTesting.php new file mode 100644 index 0000000000..b151957f9b --- /dev/null +++ b/tests/phpunit/includes/externalstore/ExternalStoreForTesting.php @@ -0,0 +1,44 @@ + [ + '200' => 'Hello', + '300' => [ + 'Hello', 'World', + ], + ], + ]; + + /** + * Fetch data from given URL + * @param string $url An url of the form FOO://cluster/id or FOO://cluster/id/itemid. + * @return mixed + */ + public function fetchFromURL( $url ) { + // Based on ExternalStoreDB + $path = explode( '/', $url ); + $cluster = $path[2]; + $id = $path[3]; + if ( isset( $path[4] ) ) { + $itemID = $path[4]; + } else { + $itemID = false; + } + + if ( !isset( $this->data[$cluster][$id] ) ) { + return null; + } + + if ( $itemID !== false + && is_array( $this->data[$cluster][$id] ) + && isset( $this->data[$cluster][$id][$itemID] ) + ) { + return $this->data[$cluster][$id][$itemID]; + } + + return $this->data[$cluster][$id]; + } + +} diff --git a/tests/phpunit/includes/externalstore/ExternalStoreTest.php b/tests/phpunit/includes/externalstore/ExternalStoreTest.php index a365c4de2e..7ca38749fa 100644 --- a/tests/phpunit/includes/externalstore/ExternalStoreTest.php +++ b/tests/phpunit/includes/externalstore/ExternalStoreTest.php @@ -1,31 +1,39 @@ setMwGlobals( 'wgExternalStores', false ); + public function testExternalFetchFromURL_noExternalStores() { + $this->setService( + 'ExternalStoreFactory', + new ExternalStoreFactory( [] ) + ); $this->assertFalse( - ExternalStore::fetchFromURL( 'FOO://cluster1/200' ), + ExternalStore::fetchFromURL( 'ForTesting://cluster1/200' ), 'Deny if wgExternalStores is not set to a non-empty array' ); + } - $this->setMwGlobals( 'wgExternalStores', [ 'FOO' ] ); + /** + * @covers ExternalStore::fetchFromURL + */ + public function testExternalFetchFromURL_someExternalStore() { + $this->setService( + 'ExternalStoreFactory', + new ExternalStoreFactory( [ 'ForTesting' ] ) + ); $this->assertEquals( - ExternalStore::fetchFromURL( 'FOO://cluster1/200' ), 'Hello', + ExternalStore::fetchFromURL( 'ForTesting://cluster1/200' ), 'Allow FOO://cluster1/200' ); $this->assertEquals( - ExternalStore::fetchFromURL( 'FOO://cluster1/300/0' ), 'Hello', + ExternalStore::fetchFromURL( 'ForTesting://cluster1/300/0' ), 'Allow FOO://cluster1/300/0' ); # Assertions for r68900 @@ -43,45 +51,3 @@ class ExternalStoreTest extends MediaWikiTestCase { ); } } - -class ExternalStoreFOO { - - protected $data = [ - 'cluster1' => [ - '200' => 'Hello', - '300' => [ - 'Hello', 'World', - ], - ], - ]; - - /** - * Fetch data from given URL - * @param string $url An url of the form FOO://cluster/id or FOO://cluster/id/itemid. - * @return mixed - */ - function fetchFromURL( $url ) { - // Based on ExternalStoreDB - $path = explode( '/', $url ); - $cluster = $path[2]; - $id = $path[3]; - if ( isset( $path[4] ) ) { - $itemID = $path[4]; - } else { - $itemID = false; - } - - if ( !isset( $this->data[$cluster][$id] ) ) { - return null; - } - - if ( $itemID !== false - && is_array( $this->data[$cluster][$id] ) - && isset( $this->data[$cluster][$id][$itemID] ) - ) { - return $this->data[$cluster][$id][$itemID]; - } - - return $this->data[$cluster][$id]; - } -}