From c94454687f04b6000ecfd4445e27f758adf48b7e Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Thu, 13 Jun 2013 13:56:29 -0400 Subject: [PATCH] Return errors from WatchAction Currently, WatchAction::doWatch and WatchAction::doUnwatch return true always. Let's have them return a status object instead. This also cleans up the handling of Status objects in some of the API modules. Change-Id: I9dd9f0fd499c37f29fa12bcdb6142238a1f11e4d --- RELEASE-NOTES-1.22 | 1 + docs/hooks.txt | 2 ++ includes/actions/WatchAction.php | 26 +++++++++++++++++---- includes/api/ApiBase.php | 38 +++++++++++++++++++++++++++++++ includes/api/ApiCreateAccount.php | 12 +--------- includes/api/ApiDelete.php | 3 +-- includes/api/ApiImport.php | 4 ++-- includes/api/ApiProtect.php | 3 +-- includes/api/ApiUserrights.php | 3 +-- includes/api/ApiWatch.php | 8 +++---- 10 files changed, 73 insertions(+), 27 deletions(-) diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22 index 203880ef82..41eaa98417 100644 --- a/RELEASE-NOTES-1.22 +++ b/RELEASE-NOTES-1.22 @@ -185,6 +185,7 @@ production. user blocks. * (bug 48201) action=parse&text=foo now assumes wikitext if no title is given, rather than using the content model of the page "API". +* action=watch may now return errors. === Languages updated in 1.22=== diff --git a/docs/hooks.txt b/docs/hooks.txt index 46949eab25..e5444ce384 100644 --- a/docs/hooks.txt +++ b/docs/hooks.txt @@ -2425,6 +2425,7 @@ $article: article "acted on" 'UnwatchArticle': Before a watch is removed from an article. $user: user watching $page: WikiPage object to be removed +&$status: Status object to be returned if the hook returns false 'UnwatchArticleComplete': After a watch is removed from an article. $user: user that watched @@ -2674,6 +2675,7 @@ used to alter the SQL query which gets the list of wanted pages. 'WatchArticle': Before a watch is added to an article. $user: user that will watch $page: WikiPage object to be watched +&$status: Status object to be returned if the hook returns false 'WatchArticleComplete': After a watch is added to an article. $user: user that watched diff --git a/includes/actions/WatchAction.php b/includes/actions/WatchAction.php index de9e1d6975..b1c48119b6 100644 --- a/includes/actions/WatchAction.php +++ b/includes/actions/WatchAction.php @@ -87,24 +87,42 @@ class WatchAction extends FormAction { return parent::checkCanExecute( $user ); } + /** + * Watch a page + * @since 1.22 Returns Status object + * @param Title $title Page to watch/unwatch + * @param User $user User who is watching/unwatching + * @return Status + */ public static function doWatch( Title $title, User $user ) { $page = WikiPage::factory( $title ); - if ( wfRunHooks( 'WatchArticle', array( &$user, &$page ) ) ) { + $status = Status::newFatal( 'hookaborted' ); + if ( wfRunHooks( 'WatchArticle', array( &$user, &$page, &$status ) ) ) { + $status = Status::newGood(); $user->addWatch( $title ); wfRunHooks( 'WatchArticleComplete', array( &$user, &$page ) ); } - return true; + return $status; } + /** + * Unwatch a page + * @since 1.22 Returns Status + * @param Title $title Page to watch/unwatch + * @param User $user User who is watching/unwatching + * @return Status + */ public static function doUnwatch( Title $title, User $user ) { $page = WikiPage::factory( $title ); - if ( wfRunHooks( 'UnwatchArticle', array( &$user, &$page ) ) ) { + $status = Status::newFatal( 'hookaborted' ); + if ( wfRunHooks( 'UnwatchArticle', array( &$user, &$page, &$status ) ) ) { + $status = Status::newGood(); $user->removeWatch( $title ); wfRunHooks( 'UnwatchArticleComplete', array( &$user, &$page ) ); } - return true; + return $status; } /** diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php index 57287d7cdf..e6e784f2c8 100644 --- a/includes/api/ApiBase.php +++ b/includes/api/ApiBase.php @@ -1221,6 +1221,44 @@ abstract class ApiBase extends ContextSource { throw new UsageException( $description, $this->encodeParamName( $errorCode ), $httpRespCode, $extradata ); } + /** + * Throw a UsageException based on the errors in the Status object. + * + * @since 1.22 + * @param Status $status Status object + * @throws UsageException + */ + public function dieStatus( $status ) { + if ( $status->isGood() ) { + throw new MWException( 'Successful status passed to ApiBase::dieStatus' ); + } + + $errors = $status->getErrorsArray(); + if ( !$errors ) { + // No errors? Assume the warnings should be treated as errors + $errors = $status->getWarningsArray(); + } + if ( !$errors ) { + // Still no errors? Punt + $errors = array( array( 'unknownerror-nocode' ) ); + } + + // Cannot use dieUsageMsg() because extensions might return custom + // error messages. + if ( $errors[0] instanceof Message ) { + $msg = $errors[0]; + $code = $msg->getKey(); + } else { + $code = array_shift( $errors[0] ); + $msg = wfMessage( $code, $errors[0] ); + } + if ( isset( ApiBase::$messageMap[$code] ) ) { + // Translate message to code, for backwards compatability + $code = ApiBase::$messageMap[$code]['code']; + } + $this->dieUsage( $msg->inLanguage( 'en' )->useDatabase( false )->plain(), $code ); + } + /** * Array that maps message keys to error messages. $1 and friends are replaced. */ diff --git a/includes/api/ApiCreateAccount.php b/includes/api/ApiCreateAccount.php index 59ff324792..9464d49a2a 100644 --- a/includes/api/ApiCreateAccount.php +++ b/includes/api/ApiCreateAccount.php @@ -128,17 +128,7 @@ class ApiCreateAccount extends ApiBase { $result['result'] = 'needtoken'; } elseif ( !$status->isOK() ) { // There was an error. Die now. - // Cannot use dieUsageMsg() directly because extensions - // might return custom error messages. - $errors = $status->getErrorsArray(); - if ( $errors[0] instanceof Message ) { - $code = 'aborted'; - $desc = $errors[0]; - } else { - $code = array_shift( $errors[0] ); - $desc = wfMessage( $code, $errors[0] ); - } - $this->dieUsage( $desc, $code ); + $this->dieStatus( $status ); } elseif ( !$status->isGood() ) { // Status is not good, but OK. This means warnings. $result['result'] = 'warning'; diff --git a/includes/api/ApiDelete.php b/includes/api/ApiDelete.php index d1f0806e71..aea10482be 100644 --- a/includes/api/ApiDelete.php +++ b/includes/api/ApiDelete.php @@ -61,8 +61,7 @@ class ApiDelete extends ApiBase { $this->dieUsageMsg( $status[0] ); } if ( !$status->isGood() ) { - $errors = $status->getErrorsArray(); - $this->dieUsageMsg( $errors[0] ); // We don't care about multiple errors, just report one of them + $this->dieStatus( $status ); } // Deprecated parameters diff --git a/includes/api/ApiImport.php b/includes/api/ApiImport.php index 56af76f93e..f48a822e36 100644 --- a/includes/api/ApiImport.php +++ b/includes/api/ApiImport.php @@ -57,7 +57,7 @@ class ApiImport extends ApiBase { $source = ImportStreamSource::newFromUpload( 'xml' ); } if ( !$source->isOK() ) { - $this->dieUsageMsg( $source->getErrorsArray() ); + $this->dieStatus( $source ); } $importer = new WikiImporter( $source->value ); @@ -67,7 +67,7 @@ class ApiImport extends ApiBase { if ( isset( $params['rootpage'] ) ) { $statusRootPage = $importer->setTargetRootPage( $params['rootpage'] ); if ( !$statusRootPage->isGood() ) { - $this->dieUsageMsg( $statusRootPage->getErrorsArray() ); + $this->dieStatus( $statusRootPage ); } } $reporter = new ApiImportReporter( diff --git a/includes/api/ApiProtect.php b/includes/api/ApiProtect.php index 503c692060..7830c8b4dc 100644 --- a/includes/api/ApiProtect.php +++ b/includes/api/ApiProtect.php @@ -103,8 +103,7 @@ class ApiProtect extends ApiBase { $status = $pageObj->doUpdateRestrictions( $protections, $expiryarray, $cascade, $params['reason'], $this->getUser() ); if ( !$status->isOK() ) { - $errors = $status->getErrorsArray(); - $this->dieUsageMsg( $errors[0] ); + $this->dieStatus( $status ); } $res = array( 'title' => $titleObj->getPrefixedText(), diff --git a/includes/api/ApiUserrights.php b/includes/api/ApiUserrights.php index 870201e703..7d308285df 100644 --- a/includes/api/ApiUserrights.php +++ b/includes/api/ApiUserrights.php @@ -66,8 +66,7 @@ class ApiUserrights extends ApiBase { $form->setContext( $this->getContext() ); $status = $form->fetchUser( $params['user'] ); if ( !$status->isOK() ) { - $errors = $status->getErrorsArray(); - $this->dieUsageMsg( $errors[0] ); + $this->dieStatus( $status ); } else { $user = $status->value; } diff --git a/includes/api/ApiWatch.php b/includes/api/ApiWatch.php index 3e51299f6a..e001be3d04 100644 --- a/includes/api/ApiWatch.php +++ b/includes/api/ApiWatch.php @@ -57,19 +57,19 @@ class ApiWatch extends ApiBase { if ( $params['unwatch'] ) { $res['unwatched'] = ''; $res['message'] = $this->msg( 'removedwatchtext', $title->getPrefixedText() )->title( $title )->parseAsBlock(); - $success = UnwatchAction::doUnwatch( $title, $user ); + $status = UnwatchAction::doUnwatch( $title, $user ); } else { $res['watched'] = ''; $res['message'] = $this->msg( 'addedwatchtext', $title->getPrefixedText() )->title( $title )->parseAsBlock(); - $success = WatchAction::doWatch( $title, $user ); + $status = WatchAction::doWatch( $title, $user ); } if ( !is_null( $oldLang ) ) { $this->getContext()->setLanguage( $oldLang ); // Reset language to $oldLang } - if ( !$success ) { - $this->dieUsageMsg( 'hookaborted' ); + if ( !$status->isOK() ) { + $this->dieStatus( $status ); } $this->getResult()->addValue( null, $this->getModuleName(), $res ); } -- 2.20.1