Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

MCR: provide MultiContentSave hook to replace PageContentSave

Browse source 
Old hook PageContentSave only received the main slot of the article.
In order for extensions to intercept changes in all slots, they should
receive a RevisionSlots object with new content.

PageContentSave hook is still preserved for backward compatibility.

Note: parameters $iswatch and $section (from PageContentSave) are
not added into the new hook, because they are obsolete in the old hook.

Note: new hook also doesn't unnecessarily pass-by-reference (T193950),
which was an issue with the old hook.

Bug: T205982
Change-Id: I55455639c5ff6ccbf3917d85c1ab0dac847ec853
This commit is contained in:
Edward Chernenko 2018-10-15 13:37:39 +03:00
parent a50432f78c
commit 9cf8427865
3 changed files with 134 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

18
docs/hooks.txt
View file

@ -2335,6 +2335,21 @@ $oldTitle: Title object of the current (old) location
$newTitle: Title object of the new location
$status: Status object to pass error messages to
'MultiContentSave': Before an article is saved.
$renderedRevision: RenderedRevision (object) representing the planned revision.
Provides access to: (1) ParserOutput of all slots,
(2) RevisionRecord, which contains
(2a) Title of the page that will be edited,
(2b) RevisionSlots - content of all slots, including inherited slots,
where content has already been modified by PreSaveTransform.
NOTE: because this revision is not yet saved, slots don't have content ID
or address, and revision itself doesn't have an ID.
$user: UserIdentity (object): author of this new revision.
$summary: CommentStoreComment (object): user-provided edit comment/summary
(not an autosummary: will be empty if the user hasn't provided a comment).
$flags: combination of EDIT_* flags, e.g. EDIT_MINOR.
$status: if the hook is aborted, error code can be placed into this Status.
'NamespaceIsMovable': Called when determining if it is possible to pages in a
namespace.
$index: Integer; the index of the namespace being checked.
@ -2460,7 +2475,8 @@ $title: Title object
strings are deprecated.
$userLang: the user language (Language object)
'PageContentSave': Before an article is saved.
'PageContentSave': DEPRECATED since 1.35! Use MultiContentSave instead.
Before an article is saved.
[&]$wikiPage: the WikiPage (object) being saved
[&]$user: the user (object) saving the article
[&]$content: the new article content, as a Content object

32
includes/Storage/PageUpdater.php
View file

@ -692,9 +692,6 @@ class PageUpdater {
$useStashed = $this->ajaxEditStash;
}
// TODO: use this only for the legacy hook, and only if something uses the legacy hook
$wikiPage = $this->getWikiPage();
$user = $this->user;
// Prepare the update. This performs PST and generates the canonical ParserOutput.
@ -734,16 +731,29 @@ class PageUpdater {
*/
$this->derivedDataUpdater->getCanonicalParserOutput();
$mainContent = $this->derivedDataUpdater->getSlots()->getContent( SlotRecord::MAIN );
// Trigger pre-save hook (using provided edit summary)
$renderedRevision = $this->derivedDataUpdater->getRenderedRevision();
$hookStatus = Status::newGood( [] );
// TODO: replace legacy hook!
// TODO: avoid pass-by-reference, see T193950
// Check if the hook rejected the attempted save
if ( !Hooks::run( 'PageContentSave', [ &$wikiPage, &$user, &$mainContent, &$summary,
$flags & EDIT_MINOR, null, null, &$flags, &$hookStatus ] )
) {
$allowedByHook = Hooks::run( 'MultiContentSave', [
$renderedRevision, $user, $summary, $flags, $hookStatus
] );
if ( $allowedByHook && Hooks::isRegistered( 'PageContentSave' ) ) {
// Also run the legacy hook.
// TODO: avoid pass-by-reference, see T193950
// NOTE: WikiPage should only be used for the legacy hook,
// and only if something uses the legacy hook.
$mainContent = $this->derivedDataUpdater->getSlots()->getContent( SlotRecord::MAIN );
$wikiPage = $this->getWikiPage();
// Deprecated since 1.35.
$allowedByHook = Hooks::run( 'PageContentSave', [
&$wikiPage, &$user, &$mainContent, &$summary,
$flags & EDIT_MINOR, null, null, &$flags, &$hookStatus
] );
}
if ( !$allowedByHook ) {
// The hook has prevented this change from being saved.
if ( $hookStatus->isOK() ) {
// Hook returned false but didn't call fatal(); use generic message
$hookStatus->fatal( 'edit-hook-aborted' );

96
tests/phpunit/includes/Storage/PageUpdaterTest.php
View file

@ -5,12 +5,14 @@ namespace MediaWiki\Tests\Storage;
use CommentStoreComment;
use Content;
use MediaWiki\MediaWikiServices;
use MediaWiki\Revision\RenderedRevision;
use MediaWiki\Revision\RevisionRecord;
use MediaWiki\Revision\SlotRecord;
use MediaWikiTestCase;
use ParserOptions;
use RecentChange;
use Revision;
use Status;
use TextContent;
use Title;
use User;
@ -260,6 +262,100 @@ class PageUpdaterTest extends MediaWikiTestCase {
return $rev;
}
/**
* Verify that MultiContentSave hook is called by saveRevision() with correct parameters.
* @covers \MediaWiki\Storage\PageUpdater::saveRevision()
*/
public function testMultiContentSaveHook() {
$user = $this->getTestUser()->getUser();
$title = $this->getDummyTitle( __METHOD__ );
// TODO: MCR: test additional slots
$slots = [
SlotRecord::MAIN => new TextContent( 'Lorem Ipsum' )
];
// start editing non-existing page
$page = WikiPage::factory( $title );
$updater = $page->newPageUpdater( $user );
foreach ( $slots as $slot => $content ) {
$updater->setContent( $slot, $content );
}
$summary = CommentStoreComment::newUnsavedComment( 'Just a test' );
$expected = [
'user' => $user,
'title' => $title,
'slots' => $slots,
'summary' => $summary
];
$hookFired = false;
$this->setTemporaryHook( 'MultiContentSave',
function ( RenderedRevision $renderedRevision, User $user,
$summary, $flags, Status $hookStatus
) use ( &$hookFired, $expected ) {
$hookFired = true;
$this->assertSame( $expected['summary'], $summary );
$this->assertSame( EDIT_NEW, $flags );
$title = $renderedRevision->getRevision()->getPageAsLinkTarget();
$this->assertSame( $expected['title']->getFullText(), $title->getFullText() );
$slots = $renderedRevision->getRevision()->getSlots();
foreach ( $expected['slots'] as $slot => $content ) {
$this->assertSame( $content, $slots->getSlot( $slot )->getContent() );
}
// Don't abort this edit.
return true;
}
);
$rev = $updater->saveRevision( $summary );
$this->assertTrue( $hookFired, "MultiContentSave hook wasn't called." );
$this->assertNotNull( $rev,
"MultiContentSave returned true, but revision wasn't created." );
}
/**
* Verify that MultiContentSave hook can abort saveRevision() by returning false.
* @covers \MediaWiki\Storage\PageUpdater::saveRevision()
*/
public function testMultiContentSaveHookAbort() {
$user = $this->getTestUser()->getUser();
$title = $this->getDummyTitle( __METHOD__ );
// start editing non-existing page
$page = WikiPage::factory( $title );
$updater = $page->newPageUpdater( $user );
$updater->setContent( SlotRecord::MAIN, new TextContent( 'Lorem Ipsum' ) );
$summary = CommentStoreComment::newUnsavedComment( 'Just a test' );
$expectedError = 'aborted-by-test-hook';
$this->setTemporaryHook( 'MultiContentSave',
function ( RenderedRevision $renderedRevision, User $user,
$summary, $flags, Status $hookStatus
) use ( $expectedError ) {
$hookStatus->fatal( $expectedError );
// Returning false should disallow saveRevision() to continue saving this revision.
return false;
}
);
$rev = $updater->saveRevision( $summary );
$this->assertNull( $rev,
"MultiContentSave returned false, but revision was still created." );
$status = $updater->getStatus();
$this->assertFalse( $status->isOK(),
"MultiContentSave returned false, but Status is not fatal." );
$this->assertSame( $expectedError, $status->getMessage()->getKey() );
}
/**
* @covers \MediaWiki\Storage\PageUpdater::grabParentRevision()
* @covers \MediaWiki\Storage\PageUpdater::saveRevision()