There's been some discussion at mediawiki-api about what we should do
with hooks in the API. There are currently three problems:
1. hooks that are supposed to be run aren't (such as ArticleDelete and
ArticleDeleteComplete before yesterday
2. some hooks that are run can mess up (such as AlternateEdit)
3. the 'hookaborted' error the API returns doesn't include details as to
who aborted the request and why
What I, personally, consider to be the best approach is to add new hooks
to the API modules, which provide a possibility for extensions to abort
the request and override the returned result. The only example thus far
is the APIEditBeforeSave hook (includes/api/ApiEditPage.php:132). Other
hooks should then be prevented from doing UI-specific things as much as
possible. The AlternateEdit hook, for instance, has no place in an API
request, as its very purpose is to override the edit form.
I've done an analysis of which hooks are run at which API request, and
whether I think they belong there. A report of my findings (basically a
list of hooks run at a certain request) can be found at the end of this
message.
Most initialization hooks (most notably the SpecialPage and LogPage*
hooks) are unnecessary for query requests (and also for many regular UI
requests). Shouldn't we lazy-load special page names and LogPage data?
The other lists look harmless, except for action=edit, which has the
AlternateEdit hook, which doesn't belong there. AlternateEdit serves to
replace the edit form. Extensions like AssertEdit that want to do
something for every action=edit request will have to use the right hooks
(APIEditPageBeforeSave or EditPage::attemptSave or whatever).
In general, I think most problems with action=edit are caused by the
fact that ApiEditPage currently just fools EditPage with a FauxRequest
object. There's a lot of mixture of UI and edit logic in EditPage, and I
think the best solution would be to split off the edit logic into a new
class. I'll be working on that in the ApiEdit_Vodafone branch for the
next few days, and I hope I can finish it before I go away for a week on
June 4th.
Roan Kattouw (Catrope)
Initialization hooks: (run for every API request, even action=query and
action=help)
- 'AuthPluginSetup' in Setup.php:258
- SpecialPage_initlist' in SpecialPage::initList()
- 'UserLoadFromSession' in User::loadFromSession()
- 'IsTrustedProxy' in wfIsTrustedProxy()
- 'LogPageValidTypes' in Setup.php:295
- 'LogPageLogName' in Setup.php:296
- 'LogPageLogHeader' in Setup.php:297
- 'LogPageActionText' in Setup.php:298
- 'UserEffectiveGroups' in User::getEffectiveGroups()
- 'UserGetRights' in User::getRights()
action=rollback runs the following hooks, in addition to the
initialization hooks and parser-related hooks I omitted for clarity:
- 'userCan' in Title::getUserPermissionsErrorInternal()
- 'getUserPermissionsErrors' in the same function
- 'getUserPermissionsErrorsExpensive' in the same function
- 'GetBlockedStatus' in User::getBlockedStatus()
- 'PingLimiter' in User::pingLimiter()
- 'ArticleSave' in Article::doEdit()
- 'ArticlePageDataBefore' in Article::pageData()
- 'ArticlePageDataAfter' in the same function
- 'ArticleAfterFetchContent' in Article::fetchContent()
- 'RevisionInsertComplete' in Revision::insertOn()
- 'NewRevisionFromEditComplete' in Article::doEdit()
- 'UserArrayFromResult' in UserArray::newFromResult()
- 'RecentChange_save' in RecentChange::save()
- 'ArticleEditUpdatesDeleteFromRecentchanges' in Article::editUpdates()
- 'SearchUpdate' in SearchUpdate::doUpdate()
- 'ArticleSaveComplete' in Article::doEdit()
- 'ArticleRollbackComplete' in Article::commitRollback()
action=delete:
- 'userCan' in Title::getUserPermissionsErrorInternal()
- 'getUserPermissionsErrors' in the same function
- 'getUserPermissionsErrorsExpensive' in the same function
- 'GetBlockedStatus' in User::getBlockedStatus()
- 'ArticleDelete' in ApiDelete::delete()
- 'ArticlePageDataBefore' in Article::pageData()
- 'ArticlePageDataAfter' in the same function
- 'ArticleAfterFetchContent' in Article::fetchContent()
- 'UserArrayFromResult' in UserArray::newFromResult()
- 'RecentChange_save' in RecentChange::save()
- 'ArticleDeleteComplete' in ApiDelete::delete()
action=undelete:
- 'GetBlockedStatus' in User::getBlockedStatus()
- 'RevisionInsertComplete' in Revision::insertOn() (for every restored
revision)
- 'ArticleRevisionUndeleted' in PageArchive::undeleteRevisions() (for
every restored revision)
- 'userCan' in Title::getUserPermissionsErrorsInternal()
- 'getUserPermissionsErrors' in Title::getUserPermissionsErrorsInternal()
- 'ArticlePageDataBefore' in Article::pageData()
- 'ArticlePageDataAfter' in the same function
- 'ArticleAfterFetchContent' in Article::fetchContent()
- 'ArticleEditUpdatesDeleteFromRecentchanges' in Article::editUpdates()
- 'ArticleUndelete' in PageArchive::undeleteRevisions()
- 'UserArrayFromResult' in UserArray::newFromResult()
- 'RecentChange_save' in RecentChange::save()
- 'SearchUpdate' in SearchUpdate::doUpdate()
action=protect:
- 'GetBlockedStatus' in User::getBlockedStatus()
- 'userCan' in Title::getUserPermissionsErrorInternal()
- 'getUserPermissionsErrors' in the same function
- 'getUserPermissionsErrorsExpensive' in the same function
- 'ArticleProtect' in Article::updateRestrictions()
- 'RevisionInsertComplete' in Revision::insertOn()
- 'NewRevisionFromEditComplete' in Article::updateRestrictions()
- 'ArticleProtectComplete' in Article::updateRestrictions()
- 'UserArrayFromResult' in UserArray::newFromResult()
- 'RecentChange_save' in RecentChange::save()
action=block:
- 'BlockIp' in IPBlockForm::doBlock()
- 'BlockIpComplete' in IPBlockForm::doBlock()
- 'UserArrayFromResult' in UserArray::newFromResult()
- 'RecentChange_save' in RecentChange::save()
action=unblock:
- 'UserArrayFromResult' in UserArray::newFromResult
- 'RecentChange_save' in RecentChange->save
action=move:
- 'userCan' in Title->getUserPermissionsErrorsInternal
- 'getUserPermissionsErrors' in Title->getUserPermissionsErrorsInternal
- 'getUserPermissionsErrorsExpensive' in
Title->getUserPermissionsErrorsInternal
- 'GetBlockedStatus' in User->getBlockedStatus
- 'AbortMove' in Title->isValidMoveOperation
- 'RevisionInsertComplete' in Revision->insertOn (twice)
- 'NewRevisionFromEditComplete' in Title->moveToNewTitle (twice)
- 'UserArrayFromResult' in UserArray::newFromResult
- 'RecentChange_save' in RecentChange->save
- 'SearchUpdate' in SearchUpdate->doUpdate
- 'TitleMoveComplete' in Title->moveTo
Note: this used to run AbortMove twice, once in ApiMove::execute()
(where it doesn't belong) and once in Title::isValidMoveOperation()
(where it does belong)
action=edit:
- 'userCan' in Title::getUserPermissionsErrorsInternal()
- 'getUserPermissionsErrors' in Title::>getUserPermissionsErrorsInternal()
- 'getUserPermissionsErrorsExpensive' in
Title::getUserPermissionsErrorsInternal()
- 'GetBlockedStatus' in User::getBlockedStatus()
- 'AlternateEdit' in ApiEditPage::execute()
- 'APIEditBeforeSave' in ApiEditPage::execute()
- 'EditPage::attemptSave' in EditPage::internalAttemptSave()
- 'EditFilter' in EditPage::internalAttemptSave()
- 'PingLimiter' in User::pingLimiter()
- 'ArticlePageDataBefore' in Article::pageData()
- 'ArticlePageDataAfter' in Article::pageData()
- 'ArticleAfterFetchContent' in Article::fetchContent()
- 'EditFilterMerged' in EditPage::internalAttemptSave()
- 'ArticleSave' in Article::doEdit()
- 'RevisionInsertComplete' in Revision::insertOn()
- 'NewRevisionFromEditComplete' in Article::doEdit()
- 'UserArrayFromResult' in UserArray::newFromResult()
- 'RecentChange_save' in RecentChange::save()
- 'ArticleEditUpdatesDeleteFromRecentchanges' in Article::editUpdates()
- 'ArticleSaveComplete' in Article::doEdit()
- 'ArticleUpdateBeforeRedirect' in Article::updateArticle()
- 'GetLocalURL' in Title::getLocalURL()
- 'GetFullURL' in Title::getFullURL()
- 'SearchUpdate' in SearchUpdate::doUpdate()