[ExternalStore] Various cleanups to ExternalStore class.

* Removed redundant $wgExternalStores check
* Fixed broken (though unused) insert() function
* Added more documentation
* Other small cleanups

Change-Id: I0be5d9ebecd58b4f5673a6205f46bb7368d889c8

diff --git a/includes/externalstore/ExternalStore.php b/includes/externalstore/ExternalStore.php
index 1b7c29d..55c1acf 100644 (file)
--- a/includes/externalstore/ExternalStore.php
+++ b/includes/externalstore/ExternalStore.php
@@ -1,6 +1,10 @@
 <?php
 /**
- * Data storage in external repositories.
+ * @defgroup ExternalStorage ExternalStorage
+ */
+
+/**
+ * Interface for data storage in external repositories.
  *
  * 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
@@ -21,11 +25,14 @@
  */
 
 /**
- * @defgroup ExternalStorage ExternalStorage
- */
-
-/**
- * Constructor class for data kept in external repositories
+ * Constructor class for data kept in external repositories.
+ *
+ * Objects in external stores are defined by a special URL. The URL is of
+ * the form "<store protocal>://<location>/<object name>". When an object
+ * is inserted into a store, the calling code uses a partial URL of the form
+ * "<store protocal>://<location>" and receives the full object URL on success.
+ * This is useful since object names can be sequential IDs, UUIDs, or hashes.
+ * Callers are not responsible for unique name generation.
  *
  * External repositories might be populated by maintenance/async
  * scripts, thus partial moving of data may be possible, as well
@@ -34,13 +41,23 @@
  * @ingroup ExternalStorage
  */
 class ExternalStore {
-       var $mParams;
-
        /**
-        * @param $params array
+        * Get an external store object of the given type, with the given parameters
+        *
+        * @param $proto String: type of external storage, should be a value in $wgExternalStores
+        * @param $params Array: associative array of parameters for the ExternalStore object.
+        * @return ExternalStore|bool ExternalStore class or false on error
         */
-       function __construct( $params = array() ) {
-               $this->mParams = $params;
+       public static function getStoreObject( $proto, array $params = array() ) {
+               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 MWInit::classExists( $class ) ? new $class( $params ) : false;
        }
 
        /**
@@ -49,23 +66,16 @@ class ExternalStore {
         * @param $url String: The URL of the text to get
         * @param $params Array: associative array of parameters for the ExternalStore object.
         * @return string|bool The text stored or false on error
+        * @throws MWException
         */
-       static function fetchFromURL( $url, $params = array() ) {
-               global $wgExternalStores;
-
-               if( !$wgExternalStores ) {
-                       return false;
-               }
-
+       public static function fetchFromURL( $url, array $params = array() ) {
                $parts = explode( '://', $url, 2 );
-
                if ( count( $parts ) != 2 ) {
-                       return false;
+                       return false; // invalid URL
                }
 
                list( $proto, $path ) = $parts;
-
-               if ( $path == '' ) { // Bad URL
+               if ( $path == '' ) { // bad URL
                        return false;
                }
 
@@ -77,33 +87,6 @@ class ExternalStore {
                return $store->fetchFromURL( $url );
        }
 
-       /**
-        * Get an external store object of the given type, with the given parameters
-        *
-        * @param $proto String: type of external storage, should be a value in $wgExternalStores
-        * @param $params Array: associative array of parameters for the ExternalStore object.
-        * @return ExternalStore|bool ExternalStore class or false on error
-        */
-       static function getStoreObject( $proto, $params = array() ) {
-               global $wgExternalStores;
-               if( !$wgExternalStores ) {
-                       return false;
-               }
-
-               /* Protocol not enabled */
-               if( !in_array( $proto, $wgExternalStores ) ) {
-                       return false;
-               }
-
-               $class = 'ExternalStore' . ucfirst( $proto );
-               /* Any custom modules should be added to $wgAutoLoadClasses for on-demand loading */
-               if( !MWInit::classExists( $class ) ) {
-                       return false;
-               }
-
-               return new $class($params);
-       }
-
        /**
         * Store a data item to an external store, identified by a partial URL
         * The protocol part is used to identify the class, the rest is passed to the
@@ -112,14 +95,24 @@ class ExternalStore {
         * @param $data
         * @param $params array
         * @return string|bool The URL of the stored data item, or false on error
+        * @throws MWException
         */
-       static function insert( $url, $data, $params = array() ) {
-               list( $proto, $params ) = explode( '://', $url, 2 );
+       public static function insert( $url, $data, array $params = array() ) {
+               $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 ) {
                        return false;
                } else {
-                       return $store->store( $params, $data );
+                       return $store->store( $path, $data );
                }
        }
 
@@ -129,42 +122,41 @@ class ExternalStore {
         * itself. It also fails-over to the next possible clusters.
         *
         * @param $data String
-        * @param $storageParams Array: associative array of parameters for the ExternalStore object.
-        * @throws MWException|DBConnectionError|DBQueryError
+        * @param $params Array: associative array of parameters for the ExternalStore object.
         * @return string|bool The URL of the stored data item, or false on error
+        * @throws MWException
         */
-       public static function insertToDefault( $data, $storageParams = array() ) {
+       public static function insertToDefault( $data, array $params = array() ) {
                global $wgDefaultExternalStore;
-               $tryStores = (array)$wgDefaultExternalStore;
+
                $error = false;
+               $tryStores = (array)$wgDefaultExternalStore;
                while ( count( $tryStores ) > 0 ) {
-                       $index = mt_rand(0, count( $tryStores ) - 1);
+                       $index = mt_rand( 0, count( $tryStores ) - 1 );
                        $storeUrl = $tryStores[$index];
                        wfDebug( __METHOD__.": trying $storeUrl\n" );
-                       list( $proto, $params ) = explode( '://', $storeUrl, 2 );
-                       $store = self::getStoreObject( $proto, $storageParams );
+                       list( $proto, $path ) = explode( '://', $storeUrl, 2 );
+                       $store = self::getStoreObject( $proto, $params );
                        if ( $store === false ) {
                                throw new MWException( "Invalid external storage protocol - $storeUrl" );
                        }
                        try {
-                               $url = $store->store( $params, $data ); // Try to save the object
-                       } catch ( DBConnectionError $error ) {
-                               $url = false;
-                       } catch( DBQueryError $error ) {
+                               $url = $store->store( $path, $data ); // Try to save the object
+                       } catch ( MWException $error ) {
                                $url = false;
                        }
-                       if ( $url ) {
+                       if ( strlen( $url ) ) {
                                return $url; // Done!
                        } else {
                                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" );
+                               wfDebugLog( 'ExternalStorage',
+                                       "Unable to store text to external storage $storeUrl" );
                        }
                }
                // All stores failed
                if ( $error ) {
-                       // Rethrow the last connection error
-                       throw $error;
+                       throw $error; // rethrow the last error
                } else {
                        throw new MWException( "Unable to store text to external storage" );
                }
@@ -173,8 +165,8 @@ class ExternalStore {
        /**
         * @param $data
         * @param $wiki
-        *
-        * @return string
+        * @return string|bool The URL of the stored data item, or false on error
+        * @throws MWException
         */
        public static function insertToForeignDefault( $data, $wiki ) {
                return self::insertToDefault( $data, array( 'wiki' => $wiki ) );