Merge "Introduce ExternalStoreFactory"

diff --git a/autoload.php b/autoload.php
index edac2c5..a998e9a 100644 (file)
--- 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 0d010b4..b39c8a4 100644 (file)
--- 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 0496b67..ae88d37 100644 (file)
--- 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 1563baf..3beab29 100644 (file)
--- 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 (file)
index 0000000..940fb2e
--- /dev/null
+++ b/includes/externalstore/ExternalStoreFactory.php
@@ -0,0 +1,42 @@
+<?php
+/**
+ * @defgroup ExternalStorage ExternalStorage
+ */
+
+/**
+ * @ingroup ExternalStorage
+ */
+class ExternalStoreFactory {
+
+       /**
+        * @var array
+        */
+       private $externalStores;
+
+       /**
+        * @param array $externalStores See $wgExternalStores
+        */
+       public function __construct( array $externalStores ) {
+               $this->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 f7bf7a6..993f8d3 100644 (file)
--- 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 (file)
index 0000000..a0bac63
--- /dev/null
+++ b/tests/phpunit/includes/externalstore/ExternalStoreFactoryTest.php
@@ -0,0 +1,39 @@
+<?php
+
+/**
+ * @covers ExternalStoreFactory
+ */
+class ExternalStoreFactoryTest extends PHPUnit_Framework_TestCase {
+
+       public function testExternalStoreFactory_noStores() {
+               $factory = new ExternalStoreFactory( [] );
+               $this->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 (file)
index 0000000..b151957
--- /dev/null
+++ b/tests/phpunit/includes/externalstore/ExternalStoreForTesting.php
@@ -0,0 +1,44 @@
+<?php
+
+class ExternalStoreForTesting {
+
+       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
+        */
+       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 a365c4d..7ca3874 100644 (file)
--- a/tests/phpunit/includes/externalstore/ExternalStoreTest.php
+++ b/tests/phpunit/includes/externalstore/ExternalStoreTest.php
@@ -1,31 +1,39 @@
 <?php
-/**
- * External Store tests
- */
 
 class ExternalStoreTest extends MediaWikiTestCase {
 
        /**
         * @covers ExternalStore::fetchFromURL
         */
-       public function testExternalFetchFromURL() {
-               $this->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];
-       }
-}