Aaron Schulz has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/311913

Change subject: Simplify and clean up FileBackend exceptions
......................................................................

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
---
M autoload.php
M includes/filebackend/FileBackendGroup.php
M includes/filebackend/FileBackendMultiWrite.php
M includes/filebackend/FileBackendStore.php
M includes/filebackend/FileOp.php
M includes/libs/filebackend/FileBackend.php
A includes/libs/filebackend/FileBackendError.php
D includes/libs/filebackend/FileBackendException.php
8 files changed, 31 insertions(+), 40 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/13/311913/1

diff --git a/autoload.php b/autoload.php
index 1e98f63..c345e02 100644
--- a/autoload.php
+++ b/autoload.php
@@ -456,8 +456,7 @@
        '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 0bae5ff..261b74a 100644
--- a/includes/filebackend/FileBackendGroup.php
+++ b/includes/filebackend/FileBackendGroup.php
@@ -114,18 +114,18 @@
         *
         * @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 @@
         *
         * @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'] ) ) {
@@ -180,11 +180,11 @@
         *
         * @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 c1cc7bb..52b84d4 100644
--- a/includes/filebackend/FileBackendMultiWrite.php
+++ b/includes/filebackend/FileBackendMultiWrite.php
@@ -114,7 +114,7 @@
                        }
                        $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 @@
                        $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 @@
                        }
                        // 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 4e25ce7..9efec36 100644
--- a/includes/filebackend/FileBackendStore.php
+++ b/includes/filebackend/FileBackendStore.php
@@ -1197,9 +1197,10 @@
 
                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 @@
         */
        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 916366c..480ebdf 100644
--- a/includes/filebackend/FileOp.php
+++ b/includes/filebackend/FileOp.php
@@ -83,7 +83,7 @@
                        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 4ff342f..c5610c0 100644
--- a/includes/libs/filebackend/FileBackend.php
+++ b/includes/libs/filebackend/FileBackend.php
@@ -134,15 +134,15 @@
         *   - 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 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']
@@ -1492,7 +1492,7 @@
 
                $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 0000000..e233535
--- /dev/null
+++ b/includes/libs/filebackend/FileBackendError.php
@@ -0,0 +1,9 @@
+<?php
+/**
+ * File backend exception for checked exceptions (e.g. I/O errors)
+ *
+ * @ingroup FileBackend
+ * @since 1.22
+ */
+class FileBackendError extends Exception {
+}
diff --git a/includes/libs/filebackend/FileBackendException.php 
b/includes/libs/filebackend/FileBackendException.php
deleted file mode 100644
index 949bce8..0000000
--- a/includes/libs/filebackend/FileBackendException.php
+++ /dev/null
@@ -1,18 +0,0 @@
-<?php
-/**
- * Generic file backend exception for checked and unexpected (e.g. config) 
exceptions
- *
- * @ingroup FileBackend
- * @since 1.23
- */
-class FileBackendException extends Exception {
-}
-
-/**
- * File backend exception for checked exceptions (e.g. I/O errors)
- *
- * @ingroup FileBackend
- * @since 1.22
- */
-class FileBackendError extends FileBackendException {
-}

-- 
To view, visit https://gerrit.wikimedia.org/r/311913
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I735a525e0b14e518b2da5f18762e0f293064dfc2
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to