From c2acd8f642b64e9ea9785c3f7d30e7000a708f84 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Tue, 20 Sep 2016 21:44:29 -0700 Subject: [PATCH] Simplify and clean up FileBackend exceptions Use standard exceptions for unexpected errors and remove FileBackendException class, leaving FileBackendError. The later is actually intended to be caught in some cases. Change-Id: I735a525e0b14e518b2da5f18762e0f293064dfc2 --- autoload.php | 3 +-- includes/filebackend/FileBackendGroup.php | 16 ++++++++-------- includes/filebackend/FileBackendMultiWrite.php | 8 ++++---- includes/filebackend/FileBackendStore.php | 7 ++++--- includes/filebackend/FileOp.php | 2 +- includes/libs/filebackend/FileBackend.php | 8 ++++---- includes/libs/filebackend/FileBackendError.php | 9 +++++++++ .../libs/filebackend/FileBackendException.php | 18 ------------------ 8 files changed, 31 insertions(+), 40 deletions(-) create mode 100644 includes/libs/filebackend/FileBackendError.php delete mode 100644 includes/libs/filebackend/FileBackendException.php diff --git a/autoload.php b/autoload.php index 018e85eb9a..d985ee0ab0 100644 --- a/autoload.php +++ b/autoload.php @@ -456,8 +456,7 @@ $wgAutoloadLocalClasses = [ 'FileAwareNodeVisitor' => __DIR__ . '/maintenance/findDeprecated.php', 'FileBackend' => __DIR__ . '/includes/libs/filebackend/FileBackend.php', 'FileBackendDBRepoWrapper' => __DIR__ . '/includes/filerepo/FileBackendDBRepoWrapper.php', - 'FileBackendError' => __DIR__ . '/includes/libs/filebackend/FileBackendException.php', - 'FileBackendException' => __DIR__ . '/includes/libs/filebackend/FileBackendException.php', + 'FileBackendError' => __DIR__ . '/includes/libs/filebackend/FileBackendError.php', 'FileBackendGroup' => __DIR__ . '/includes/filebackend/FileBackendGroup.php', 'FileBackendMultiWrite' => __DIR__ . '/includes/filebackend/FileBackendMultiWrite.php', 'FileBackendStore' => __DIR__ . '/includes/filebackend/FileBackendStore.php', diff --git a/includes/filebackend/FileBackendGroup.php b/includes/filebackend/FileBackendGroup.php index b560e94307..d0a99d4d65 100644 --- a/includes/filebackend/FileBackendGroup.php +++ b/includes/filebackend/FileBackendGroup.php @@ -114,18 +114,18 @@ class FileBackendGroup { * * @param array $configs * @param string|null $readOnlyReason - * @throws FileBackendException + * @throws InvalidArgumentException */ protected function register( array $configs, $readOnlyReason = null ) { foreach ( $configs as $config ) { if ( !isset( $config['name'] ) ) { - throw new FileBackendException( "Cannot register a backend with no name." ); + throw new InvalidArgumentException( "Cannot register a backend with no name." ); } $name = $config['name']; if ( isset( $this->backends[$name] ) ) { - throw new FileBackendException( "Backend with name `{$name}` already registered." ); + throw new LogicException( "Backend with name `{$name}` already registered." ); } elseif ( !isset( $config['class'] ) ) { - throw new FileBackendException( "Backend with name `{$name}` has no class." ); + throw new InvalidArgumentException( "Backend with name `{$name}` has no class." ); } $class = $config['class']; @@ -147,11 +147,11 @@ class FileBackendGroup { * * @param string $name * @return FileBackend - * @throws FileBackendException + * @throws InvalidArgumentException */ public function get( $name ) { if ( !isset( $this->backends[$name] ) ) { - throw new FileBackendException( "No backend defined with the name `$name`." ); + throw new InvalidArgumentException( "No backend defined with the name `$name`." ); } // Lazy-load the actual backend instance if ( !isset( $this->backends[$name]['instance'] ) ) { @@ -181,11 +181,11 @@ class FileBackendGroup { * * @param string $name * @return array - * @throws FileBackendException + * @throws InvalidArgumentException */ public function config( $name ) { if ( !isset( $this->backends[$name] ) ) { - throw new FileBackendException( "No backend defined with the name `$name`." ); + throw new InvalidArgumentException( "No backend defined with the name `$name`." ); } $class = $this->backends[$name]['class']; diff --git a/includes/filebackend/FileBackendMultiWrite.php b/includes/filebackend/FileBackendMultiWrite.php index c1cc7bb35e..52b84d4e66 100644 --- a/includes/filebackend/FileBackendMultiWrite.php +++ b/includes/filebackend/FileBackendMultiWrite.php @@ -114,7 +114,7 @@ class FileBackendMultiWrite extends FileBackend { } $name = $config['name']; if ( isset( $namesUsed[$name] ) ) { // don't break FileOp predicates - throw new FileBackendError( "Two or more backends defined with the name $name." ); + throw new LogicException( "Two or more backends defined with the name $name." ); } $namesUsed[$name] = 1; // Alter certain sub-backend settings for sanity @@ -124,7 +124,7 @@ class FileBackendMultiWrite extends FileBackend { $config['wikiId'] = $this->wikiId; // use the proxy backend wiki ID if ( !empty( $config['isMultiMaster'] ) ) { if ( $this->masterIndex >= 0 ) { - throw new FileBackendError( 'More than one master backend defined.' ); + throw new LogicException( 'More than one master backend defined.' ); } $this->masterIndex = $index; // this is the "master" $config['fileJournal'] = $this->fileJournal; // log under proxy backend @@ -134,13 +134,13 @@ class FileBackendMultiWrite extends FileBackend { } // Create sub-backend object if ( !isset( $config['class'] ) ) { - throw new FileBackendError( 'No class given for a backend config.' ); + throw new InvalidArgumentException( '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 FileBackendError( 'No master backend defined.' ); + throw new LogicException( 'No master backend defined.' ); } if ( $this->readIndex < 0 ) { $this->readIndex = $this->masterIndex; // default diff --git a/includes/filebackend/FileBackendStore.php b/includes/filebackend/FileBackendStore.php index 4e25ce7247..9efec36c5d 100644 --- a/includes/filebackend/FileBackendStore.php +++ b/includes/filebackend/FileBackendStore.php @@ -1197,9 +1197,10 @@ abstract class FileBackendStore extends FileBackend { foreach ( $fileOpHandles as $fileOpHandle ) { if ( !( $fileOpHandle instanceof FileBackendStoreOpHandle ) ) { - throw new FileBackendError( "Given a non-FileBackendStoreOpHandle object." ); + throw new InvalidArgumentException( "Got a non-FileBackendStoreOpHandle object." ); } elseif ( $fileOpHandle->backend->getName() !== $this->getName() ) { - throw new FileBackendError( "Given a FileBackendStoreOpHandle for the wrong backend." ); + throw new InvalidArgumentException( + "Got a FileBackendStoreOpHandle for the wrong backend." ); } } $res = $this->doExecuteOpHandlesInternal( $fileOpHandles ); @@ -1220,7 +1221,7 @@ abstract class FileBackendStore extends FileBackend { */ protected function doExecuteOpHandlesInternal( array $fileOpHandles ) { if ( count( $fileOpHandles ) ) { - throw new FileBackendError( "This backend supports no asynchronous operations." ); + throw new LogicException( "Backend does not support asynchronous operations." ); } return []; diff --git a/includes/filebackend/FileOp.php b/includes/filebackend/FileOp.php index 916366c8db..480ebdf051 100644 --- a/includes/filebackend/FileOp.php +++ b/includes/filebackend/FileOp.php @@ -83,7 +83,7 @@ abstract class FileOp { if ( isset( $params[$name] ) ) { $this->params[$name] = $params[$name]; } else { - throw new FileBackendError( "File operation missing parameter '$name'." ); + throw new InvalidArgumentException( "File operation missing parameter '$name'." ); } } foreach ( $optional as $name ) { diff --git a/includes/libs/filebackend/FileBackend.php b/includes/libs/filebackend/FileBackend.php index 1317b65a94..0b9eee0624 100644 --- a/includes/libs/filebackend/FileBackend.php +++ b/includes/libs/filebackend/FileBackend.php @@ -139,15 +139,15 @@ abstract class FileBackend { * - concurrency : How many file operations can be done in parallel. * - tmpDirectory : Directory to use for temporary files. If this is not set or null, * then the backend will try to discover a usable temporary directory. - * @throws FileBackendException + * @throws InvalidArgumentException */ public function __construct( array $config ) { $this->name = $config['name']; $this->wikiId = $config['wikiId']; // e.g. "my_wiki-en_" if ( !preg_match( '!^[a-zA-Z0-9-_]{1,255}$!', $this->name ) ) { - throw new FileBackendException( "Backend name '{$this->name}' is invalid." ); + throw new InvalidArgumentException( "Backend name '{$this->name}' is invalid." ); } elseif ( !is_string( $this->wikiId ) ) { - throw new FileBackendException( "Backend wiki ID not provided for '{$this->name}'." ); + throw new InvalidArgumentException( "Backend wiki ID not provided for '{$this->name}'." ); } $this->lockManager = isset( $config['lockManager'] ) ? $config['lockManager'] @@ -1498,7 +1498,7 @@ abstract class FileBackend { $type = strtolower( $type ); if ( !in_array( $type, [ 'inline', 'attachment' ] ) ) { - throw new FileBackendError( "Invalid Content-Disposition type '$type'." ); + throw new InvalidArgumentException( "Invalid Content-Disposition type '$type'." ); } $parts[] = $type; diff --git a/includes/libs/filebackend/FileBackendError.php b/includes/libs/filebackend/FileBackendError.php new file mode 100644 index 0000000000..e233535102 --- /dev/null +++ b/includes/libs/filebackend/FileBackendError.php @@ -0,0 +1,9 @@ +