filebackend: exception handling cleanups
authorAaron Schulz <aschulz@wikimedia.org>
Sat, 23 Nov 2013 20:28:34 +0000 (12:28 -0800)
committerBryan Davis <bd808@wikimedia.org>
Sun, 8 Dec 2013 06:22:07 +0000 (23:22 -0700)
* Split out FileBackendException class and reduced direct use of MWException

Change-Id: I325c1798b6d90972c12a5dccc37989af34d857f3

includes/AutoLoader.php
includes/filebackend/FSFile.php
includes/filebackend/FileBackend.php
includes/filebackend/FileBackendGroup.php
includes/filebackend/FileBackendMultiWrite.php
includes/filebackend/FileBackendStore.php
includes/filebackend/FileOp.php
includes/filebackend/SwiftFileBackend.php

index 6ee62e0..820041c 100644 (file)
@@ -533,6 +533,7 @@ $wgAutoloadLocalClasses = array(
        'FileBackendGroup' => 'includes/filebackend/FileBackendGroup.php',
        'FileBackend' => 'includes/filebackend/FileBackend.php',
        'FileBackendError' => 'includes/filebackend/FileBackend.php',
+       'FileBackendException' => 'includes/filebackend/FileBackend.php',
        'FileBackendStore' => 'includes/filebackend/FileBackendStore.php',
        'FileBackendStoreShardListIterator' => 'includes/filebackend/FileBackendStore.php',
        'FileBackendStoreShardDirIterator' => 'includes/filebackend/FileBackendStore.php',
index 3a0aa97..047aefd 100644 (file)
@@ -37,12 +37,8 @@ class FSFile {
         * Sets up the file object
         *
         * @param string $path Path to temporary file on local disk
-        * @throws MWException
         */
        public function __construct( $path ) {
-               if ( FileBackend::isStoragePath( $path ) ) {
-                       throw new MWException( __METHOD__ . " given storage path `$path`." );
-               }
                $this->path = $path;
        }
 
index a6eda18..0e9a41d 100644 (file)
@@ -126,12 +126,12 @@ abstract class FileBackend {
         *   - parallelize : When to do file operations in parallel (when possible).
         *                   Allowed values are "implicit", "explicit" and "off".
         *   - concurrency : How many file operations can be done in parallel.
-        * @throws MWException
+        * @throws FileBackendException
         */
        public function __construct( array $config ) {
                $this->name = $config['name'];
                if ( !preg_match( '!^[a-zA-Z0-9-_]{1,255}$!', $this->name ) ) {
-                       throw new MWException( "Backend name `{$this->name}` is invalid." );
+                       throw new FileBackendException( "Backend name `{$this->name}` is invalid." );
                }
                $this->wikiId = isset( $config['wikiId'] )
                        ? $config['wikiId']
@@ -1360,7 +1360,7 @@ abstract class FileBackend {
         *
         * @param string $type One of (attachment, inline)
         * @param string $filename Suggested file name (should not contain slashes)
-        * @throws MWException
+        * @throws FileBackendError
         * @return string
         * @since 1.20
         */
@@ -1369,7 +1369,7 @@ abstract class FileBackend {
 
                $type = strtolower( $type );
                if ( !in_array( $type, array( 'inline', 'attachment' ) ) ) {
-                       throw new MWException( "Invalid Content-Disposition type '$type'." );
+                       throw new FileBackendError( "Invalid Content-Disposition type '$type'." );
                }
                $parts[] = $type;
 
@@ -1416,8 +1416,19 @@ abstract class FileBackend {
 }
 
 /**
+ * Generic file backend exception for checked and unexpected (e.g. config) exceptions
+ *
+ * @ingroup FileBackend
+ * @since 1.23
+ */
+class FileBackendException extends MWException {
+}
+
+/**
+ * File backend exception for checked exceptions (e.g. I/O errors)
+ *
  * @ingroup FileBackend
  * @since 1.22
  */
-class FileBackendError extends MWException {
+class FileBackendError extends FileBackendException {
 }
index 491424b..416fe84 100644 (file)
@@ -113,18 +113,18 @@ class FileBackendGroup {
         * Register an array of file backend configurations
         *
         * @param array $configs
-        * @throws MWException
+        * @throws FileBackendException
         */
        protected function register( array $configs ) {
                foreach ( $configs as $config ) {
                        if ( !isset( $config['name'] ) ) {
-                               throw new MWException( "Cannot register a backend with no name." );
+                               throw new FileBackendException( "Cannot register a backend with no name." );
                        }
                        $name = $config['name'];
                        if ( isset( $this->backends[$name] ) ) {
-                               throw new MWException( "Backend with name `{$name}` already registered." );
+                               throw new FileBackendException( "Backend with name `{$name}` already registered." );
                        } elseif ( !isset( $config['class'] ) ) {
-                               throw new MWException( "Cannot register backend `{$name}` with no class." );
+                               throw new FileBackendException( "Backend with name `{$name}` has no class." );
                        }
                        $class = $config['class'];
 
@@ -142,11 +142,11 @@ class FileBackendGroup {
         *
         * @param string $name
         * @return FileBackend
-        * @throws MWException
+        * @throws FileBackendException
         */
        public function get( $name ) {
                if ( !isset( $this->backends[$name] ) ) {
-                       throw new MWException( "No backend defined with the name `$name`." );
+                       throw new FileBackendException( "No backend defined with the name `$name`." );
                }
                // Lazy-load the actual backend instance
                if ( !isset( $this->backends[$name]['instance'] ) ) {
@@ -163,11 +163,11 @@ class FileBackendGroup {
         *
         * @param string $name
         * @return array
-        * @throws MWException
+        * @throws FileBackendException
         */
        public function config( $name ) {
                if ( !isset( $this->backends[$name] ) ) {
-                       throw new MWException( "No backend defined with the name `$name`." );
+                       throw new FileBackendException( "No backend defined with the name `$name`." );
                }
                $class = $this->backends[$name]['class'];
 
index b3c46c6..612b19b 100644 (file)
@@ -92,7 +92,7 @@ class FileBackendMultiWrite extends FileBackend {
         *   - noPushDirConts : (hack) Only apply directory functions to the master backend.
         *
         * @param array $config
-        * @throws MWException
+        * @throws FileBackendError
         */
        public function __construct( array $config ) {
                parent::__construct( $config );
@@ -119,7 +119,7 @@ class FileBackendMultiWrite extends FileBackend {
                        }
                        $name = $config['name'];
                        if ( isset( $namesUsed[$name] ) ) { // don't break FileOp predicates
-                               throw new MWException( "Two or more backends defined with the name $name." );
+                               throw new FileBackendError( "Two or more backends defined with the name $name." );
                        }
                        $namesUsed[$name] = 1;
                        // Alter certain sub-backend settings for sanity
@@ -129,20 +129,20 @@ class FileBackendMultiWrite extends FileBackend {
                        $config['lockManager'] = 'nullLockManager'; // lock under proxy backend
                        if ( !empty( $config['isMultiMaster'] ) ) {
                                if ( $this->masterIndex >= 0 ) {
-                                       throw new MWException( 'More than one master backend defined.' );
+                                       throw new FileBackendError( 'More than one master backend defined.' );
                                }
                                $this->masterIndex = $index; // this is the "master"
                                $config['fileJournal'] = $this->fileJournal; // log under proxy backend
                        }
                        // Create sub-backend object
                        if ( !isset( $config['class'] ) ) {
-                               throw new MWException( 'No class given for a backend config.' );
+                               throw new FileBackendError( 'No class given for a backend config.' );
                        }
                        $class = $config['class'];
                        $this->backends[$index] = new $class( $config );
                }
                if ( $this->masterIndex < 0 ) { // need backends and must have a master
-                       throw new MWException( 'No master backend defined.' );
+                       throw new FileBackendError( 'No master backend defined.' );
                }
        }
 
index 2f3e0e2..fe3a068 100644 (file)
@@ -965,7 +965,7 @@ abstract class FileBackendStore extends FileBackend {
         *
         * @param array $ops Same format as doOperations()
         * @return array List of FileOp objects
-        * @throws MWException
+        * @throws FileBackendError
         */
        final public function getOperationsInternal( array $ops ) {
                $supportedOps = array(
@@ -989,7 +989,7 @@ abstract class FileBackendStore extends FileBackend {
                                // Append the FileOp class
                                $performOps[] = new $class( $this, $params );
                        } else {
-                               throw new MWException( "Operation '$opName' is not supported." );
+                               throw new FileBackendError( "Operation '$opName' is not supported." );
                        }
                }
 
@@ -1091,7 +1091,7 @@ abstract class FileBackendStore extends FileBackend {
                // Perform the sync-only ops and build up op handles for the async ops...
                foreach ( $ops as $index => $params ) {
                        if ( !in_array( $params['op'], $supportedOps ) ) {
-                               throw new MWException( "Operation '{$params['op']}' is not supported." );
+                               throw new FileBackendError( "Operation '{$params['op']}' is not supported." );
                        }
                        $method = $params['op'] . 'Internal'; // e.g. "storeInternal"
                        $subStatus = $this->$method( array( 'async' => $async ) + $params );
@@ -1133,17 +1133,18 @@ abstract class FileBackendStore extends FileBackend {
         * to the order in which the handles where given.
         *
         * @param array $fileOpHandles
-        * @throws MWException
+        * @throws FileBackendError
         * @internal param array $handles List of FileBackendStoreOpHandle objects
         * @return array Map of Status objects
         */
        final public function executeOpHandlesInternal( array $fileOpHandles ) {
                $section = new ProfileSection( __METHOD__ . "-{$this->name}" );
+
                foreach ( $fileOpHandles as $fileOpHandle ) {
                        if ( !( $fileOpHandle instanceof FileBackendStoreOpHandle ) ) {
-                               throw new MWException( "Given a non-FileBackendStoreOpHandle object." );
+                               throw new FileBackendError( "Given a non-FileBackendStoreOpHandle object." );
                        } elseif ( $fileOpHandle->backend->getName() !== $this->getName() ) {
-                               throw new MWException( "Given a FileBackendStoreOpHandle for the wrong backend." );
+                               throw new FileBackendError( "Given a FileBackendStoreOpHandle for the wrong backend." );
                        }
                }
                $res = $this->doExecuteOpHandlesInternal( $fileOpHandles );
@@ -1157,12 +1158,12 @@ abstract class FileBackendStore extends FileBackend {
        /**
         * @see FileBackendStore::executeOpHandlesInternal()
         * @param array $fileOpHandles
-        * @throws MWException
+        * @throws FileBackendError
         * @return array List of corresponding Status objects
         */
        protected function doExecuteOpHandlesInternal( array $fileOpHandles ) {
                if ( count( $fileOpHandles ) ) {
-                       throw new MWException( "This backend supports no asynchronous operations." );
+                       throw new FileBackendError( "This backend supports no asynchronous operations." );
                }
 
                return array();
index 82959d8..4e03675 100644 (file)
@@ -74,7 +74,7 @@ abstract class FileOp {
         *
         * @param FileBackendStore $backend
         * @param array $params
-        * @throws MWException
+        * @throws FileBackendError
         */
        final public function __construct( FileBackendStore $backend, array $params ) {
                $this->backend = $backend;
@@ -83,7 +83,7 @@ abstract class FileOp {
                        if ( isset( $params[$name] ) ) {
                                $this->params[$name] = $params[$name];
                        } else {
-                               throw new MWException( "File operation missing parameter '$name'." );
+                               throw new FileBackendError( "File operation missing parameter '$name'." );
                        }
                }
                foreach ( $optional as $name ) {
index dc2995f..d79ceca 100644 (file)
@@ -124,7 +124,7 @@ class SwiftFileBackend extends FileBackendStore {
        public function __construct( array $config ) {
                parent::__construct( $config );
                if ( !class_exists( 'CF_Constants' ) ) {
-                       throw new MWException( 'SwiftCloudFiles extension not installed.' );
+                       throw new FileBackendException( 'SwiftCloudFiles extension not installed.' );
                }
                // Required settings
                $this->auth = new CF_Authentication(
@@ -837,12 +837,15 @@ class SwiftFileBackend extends FileBackendStore {
         * @param string $ts
         * @param int $format Output format (TS_* constant)
         * @return string
-        * @throws MWException
+        * @throws FileBackendError
         */
        protected function convertSwiftDate( $ts, $format = TS_MW ) {
-               $timestamp = new MWTimestamp( $ts );
-
-               return $timestamp->getTimestamp( $format );
+               try {
+                       $timestamp = new MWTimestamp( $ts );
+                       return $timestamp->getTimestamp( $format );
+               } catch ( MWException $e ) {
+                       throw new FileBackendError( $e->getMessage() );
+               }
        }
 
        /**