Page MenuHomePhabricator

TitleKey should use TitleMoveCompleting hook in 1.27 instead of TitleMoveCompleted
Open, Needs TriagePublic

Description

In MediaWiki 1.27, code in MovePage.php changed regarding the hook TitleMoveComplete. This change has an impact on the extension TitleKey in rare circumstances during phpunit testing. TitleKey should use the new hook TitleMoveCompleting instead, which appears to eliminate the problem.

Details:

The 1.26 version of TitleMoveComplete was:

Hooks::run('TitleMoveComplete', ...)

In 1.27, it became:

$dbw->onTransactionIdle( function () use ( $params, $dbw ) {                             
  $dbw->setFlag( DBO_TRX );   
   Hooks::run( 'TitleMoveComplete', $params );
} );

At the same time, a new hook was introduced in 1.27, TitleMoveCompleting, that is roughly like the 1.26 version of TitleMoveComplete.

In rare circumstances, and only in a unit testing context (with phpunit), the TitleKey extension in 1.27 throws this error due to its callback function TitleKey::updateMove():

PHP Notice:  Uncommitted DB writes (transaction from DatabaseBase::query (LinkCache::addLinkObj)). in /home/wiki/wiki/wiki/includes/db/Database.php on line 3303
PHP Stack trace:
PHP   1. {main}() /home/wiki/wiki/wiki/tests/phpunit/phpunit.php:0
PHP   2. PHPUnit_TextUI_Command::main() /home/wiki/wiki/wiki/tests/phpunit/phpunit.php:262
PHP   3. PHPUnit_TextUI_Command->run() /home/wiki/wiki/wiki/vendor/phpunit/phpunit/src/TextUI/Command.php:100
PHP   4. DatabaseBase->__destruct() /home/wiki/wiki/wiki/includes/db/Database.php:0
PHP   5. trigger_error() /home/wiki/wiki/wiki/includes/db/Database.php:3303

However, if TitleKey.php is modified to use the new hook TitleMoveCompleting instead of the old hook TitleMoveComplete, this error vanishes and the TitleKey functionality still seems to work.

I would love to provide a test case that throws the database error, but for us it involves a custom extension that is quite complex. The two hooks are very close in MovePage.php and hopefully this is a nonintrusive change for the extension.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 21 2016, 7:14 PM

Suggested patch attached.

brion added a subscriber: brion.Oct 21 2016, 8:01 PM

Change 317220 had a related patch set uploaded (by Brion VIBBER):
Switch TitleKey to use TitleMoveCompleting hook

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

brion added a comment.Oct 21 2016, 8:32 PM

Gerrit patch https://gerrit.wikimedia.org/r/317220 is for the current (1.28) version.

Thank you, Brion!

Change 317220 merged by jenkins-bot:
Switch TitleKey to use TitleMoveCompleting hook

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