From 99e124da9060a13368f66fd4f4dc48defee55559 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Mon, 21 Jan 2013 15:37:23 -0800 Subject: [PATCH] [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 --- includes/externalstore/ExternalStore.php | 130 +++++++++++------------ 1 file changed, 61 insertions(+), 69 deletions(-) diff --git a/includes/externalstore/ExternalStore.php b/includes/externalstore/ExternalStore.php index 1b7c29dbbb..55c1acf676 100644 --- a/includes/externalstore/ExternalStore.php +++ b/includes/externalstore/ExternalStore.php @@ -1,6 +1,10 @@ :///". When an object + * is inserted into a store, the calling code uses a partial URL of the form + * "://" 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 ) ); -- 2.20.1