From 754b8babca9f556fedfa97ab833fcf0132028d4b Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sat, 15 Jun 2013 23:01:47 -0700 Subject: [PATCH] filebackend: throw exceptions during file iteration. * This lets calling code be far more robust rather than just silently ignoring entries due to some temporary problem. Change-Id: I3ce2ae34f6cff5e40a80b8da5688503a387ce2a6 --- includes/AutoLoader.php | 1 + includes/filebackend/FSFileBackend.php | 8 ++++---- includes/filebackend/FileBackend.php | 14 ++++++++++++++ includes/filebackend/SwiftFileBackend.php | 16 +++++++++------- 4 files changed, 28 insertions(+), 11 deletions(-) diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php index 02c92df8c5..38b27e636d 100644 --- a/includes/AutoLoader.php +++ b/includes/AutoLoader.php @@ -551,6 +551,7 @@ $wgAutoloadLocalClasses = array( # includes/filebackend 'FileBackendGroup' => 'includes/filebackend/FileBackendGroup.php', 'FileBackend' => 'includes/filebackend/FileBackend.php', + 'FileBackendError' => 'includes/filebackend/FileBackend.php', 'FileBackendStore' => 'includes/filebackend/FileBackendStore.php', 'FileBackendStoreShardListIterator' => 'includes/filebackend/FileBackendStore.php', 'FileBackendStoreShardDirIterator' => 'includes/filebackend/FileBackendStore.php', diff --git a/includes/filebackend/FSFileBackend.php b/includes/filebackend/FSFileBackend.php index dfc519275b..6d6421627e 100644 --- a/includes/filebackend/FSFileBackend.php +++ b/includes/filebackend/FSFileBackend.php @@ -854,8 +854,8 @@ abstract class FSFileBackendList implements Iterator { try { $this->iter->next(); $this->filterViaNext(); - } catch ( UnexpectedValueException $e ) { - $this->iter = null; + } catch ( UnexpectedValueException $e ) { // bad permissions? deleted? + throw new FileBackendError( "File iterator gave UnexpectedValueException." ); } ++$this->pos; } @@ -869,8 +869,8 @@ abstract class FSFileBackendList implements Iterator { try { $this->iter->rewind(); $this->filterViaNext(); - } catch ( UnexpectedValueException $e ) { - $this->iter = null; + } catch ( UnexpectedValueException $e ) { // bad permissions? deleted? + throw new FileBackendError( "File iterator gave UnexpectedValueException." ); } } diff --git a/includes/filebackend/FileBackend.php b/includes/filebackend/FileBackend.php index 29af88db64..9b4760af43 100644 --- a/includes/filebackend/FileBackend.php +++ b/includes/filebackend/FileBackend.php @@ -1061,6 +1061,8 @@ abstract class FileBackend { * * Storage backends with eventual consistency might return stale data. * + * Failures during iteration can result in FileBackendError exceptions (since 1.22). + * * @param array $params * $params include: * - dir : storage directory @@ -1076,6 +1078,8 @@ abstract class FileBackend { * * Storage backends with eventual consistency might return stale data. * + * Failures during iteration can result in FileBackendError exceptions (since 1.22). + * * @param array $params * $params include: * - dir : storage directory @@ -1096,6 +1100,8 @@ abstract class FileBackend { * * Storage backends with eventual consistency might return stale data. * + * Failures during iteration can result in FileBackendError exceptions (since 1.22). + * * @param array $params * $params include: * - dir : storage directory @@ -1111,6 +1117,8 @@ abstract class FileBackend { * * Storage backends with eventual consistency might return stale data. * + * Failures during iteration can result in FileBackendError exceptions (since 1.22). + * * @param array $params * $params include: * - dir : storage directory @@ -1379,3 +1387,9 @@ abstract class FileBackend { return $path; } } + +/** + * @ingroup FileBackend + * @since 1.22 + */ +class FileBackendError extends MWException {} diff --git a/includes/filebackend/SwiftFileBackend.php b/includes/filebackend/SwiftFileBackend.php index a57bf5917b..f3aa145981 100644 --- a/includes/filebackend/SwiftFileBackend.php +++ b/includes/filebackend/SwiftFileBackend.php @@ -926,6 +926,7 @@ class SwiftFileBackend extends FileBackendStore { * @param integer $limit Max number of items to list * @param array $params Parameters for getDirectoryList() * @return Array List of resolved paths of directories directly under $dir + * @throws FileBackendError */ public function getDirListPageInternal( $fullCont, $dir, &$after, $limit, array $params ) { $dirs = array(); @@ -933,7 +934,7 @@ class SwiftFileBackend extends FileBackendStore { return $dirs; // nothing more } - wfProfileIn( __METHOD__ . '-' . $this->name ); + $section = new ProfileSection( __METHOD__ . '-' . $this->name ); try { $container = $this->getContainer( $fullCont ); $prefix = ( $dir == '' ) ? null : "{$dir}/"; @@ -981,8 +982,8 @@ class SwiftFileBackend extends FileBackendStore { } catch ( CloudFilesException $e ) { // some other exception? $this->handleException( $e, null, __METHOD__, array( 'cont' => $fullCont, 'dir' => $dir ) ); + throw new FileBackendError( "Got " . get_class( $e ) . " exception." ); } - wfProfileOut( __METHOD__ . '-' . $this->name ); return $dirs; } @@ -1000,6 +1001,7 @@ class SwiftFileBackend extends FileBackendStore { * @param integer $limit Max number of items to list * @param array $params Parameters for getDirectoryList() * @return Array List of resolved paths of files under $dir + * @throws FileBackendError */ public function getFileListPageInternal( $fullCont, $dir, &$after, $limit, array $params ) { $files = array(); @@ -1007,7 +1009,7 @@ class SwiftFileBackend extends FileBackendStore { return $files; // nothing more } - wfProfileIn( __METHOD__ . '-' . $this->name ); + $section = new ProfileSection( __METHOD__ . '-' . $this->name ); try { $container = $this->getContainer( $fullCont ); $prefix = ( $dir == '' ) ? null : "{$dir}/"; @@ -1048,8 +1050,8 @@ class SwiftFileBackend extends FileBackendStore { } catch ( CloudFilesException $e ) { // some other exception? $this->handleException( $e, null, __METHOD__, array( 'cont' => $fullCont, 'dir' => $dir ) ); + throw new FileBackendError( "Got " . get_class( $e ) . " exception." ); } - wfProfileOut( __METHOD__ . '-' . $this->name ); return $files; } @@ -1653,7 +1655,7 @@ abstract class SwiftFileBackendList implements Iterator { * @param string $after|null * @param integer $limit * @param array $params - * @return Traversable|Array|null Returns null on failure + * @return Traversable|Array */ abstract protected function pageFromList( $container, $dir, &$after, $limit, array $params ); } @@ -1672,7 +1674,7 @@ class SwiftFileBackendDirList extends SwiftFileBackendList { /** * @see SwiftFileBackendList::pageFromList() - * @return Array|null + * @return Array */ protected function pageFromList( $container, $dir, &$after, $limit, array $params ) { return $this->backend->getDirListPageInternal( $container, $dir, $after, $limit, $params ); @@ -1693,7 +1695,7 @@ class SwiftFileBackendFileList extends SwiftFileBackendList { /** * @see SwiftFileBackendList::pageFromList() - * @return Array|null + * @return Array */ protected function pageFromList( $container, $dir, &$after, $limit, array $params ) { return $this->backend->getFileListPageInternal( $container, $dir, $after, $limit, $params ); -- 2.20.1