(bug 24620) Add types and test for LogFormatter

This change adds types to LogFormatter like "msg" so log values
(parameters for the log message) can be formated as e.g. a message in
user or content language.

This change adds tests for LogFormatter, too. Tested are the normal log
params, the log params with type and the output of the comment.

This change repairs the changes I6a56c204 and I7218a173

Change-Id: Ief3665056b3bb613ff709821306017ee7967c444

diff --git a/RELEASE-NOTES-1.21 b/RELEASE-NOTES-1.21
index b7571bb..1776d32 100644 (file)
--- a/RELEASE-NOTES-1.21
+++ b/RELEASE-NOTES-1.21
@@ -60,6 +60,7 @@ production.
 * Debug message emitted by wfDebugLog() will now be prefixed with the group
   name when its logged to the default log file. That is the case whenever the
   group has no key in wgDebugLogGroups, that will help triage the default log.
+* (bug 24620) Add types to LogFormatter
 
 === Bug fixes in 1.21 ===
 * (bug 40353) SpecialDoubleRedirect should support interwiki redirects.

diff --git a/includes/logging/LogFormatter.php b/includes/logging/LogFormatter.php
index 8c1e294..7492209 100644 (file)
--- a/includes/logging/LogFormatter.php
+++ b/includes/logging/LogFormatter.php
@@ -403,7 +403,7 @@ class LogFormatter {
                foreach ( $entry->getParameters() as $key => $value ) {
                        if ( strpos( $key, ':' ) === false ) continue;
                        list( $index, $type, $name ) = explode( ':', $key, 3 );
-                       $params[$index - 1] = $value;
+                       $params[$index - 1] = $this->formatParameterValue( $type, $value );
                }
 
                /* Message class doesn't like non consecutive numbering.
@@ -446,6 +446,78 @@ class LogFormatter {
                return $this->parsedParameters = $params;
        }
 
+       /**
+        * Formats parameters values dependent to their type
+        * @param $type string The type of the value.
+        *   Valid are currently:
+        *     * - (empty) or plain: The value is returned as-is
+        *     * raw: The value will be added to the log message
+        *            as raw parameter (e.g. no escaping)
+        *            Use this only if there is no other working
+        *            type like user-link or title-link
+        *     * msg: The value is a message-key, the output is
+        *            the message in user language
+        *     * msg-content: The value is a message-key, the output
+        *                    is the message in content language
+        *     * user: The value is a user name, e.g. for GENDER
+        *     * user-link: The value is a user name, returns a
+        *                  link for the user
+        *     * title: The value is a page title,
+        *              returns name of page
+        *     * title-link: The value is a page title,
+        *                   returns link to this page
+        *     * number: Format value as number
+        * @param $value string The parameter value that should
+        *                      be formated
+        * @return string or Message::numParam or Message::rawParam
+        *         Formated value
+        * @since 1.21
+        */
+       protected function formatParameterValue( $type, $value ) {
+               $saveLinkFlood = $this->linkFlood;
+
+               switch( strtolower( trim( $type ) ) ) {
+                       case 'raw':
+                               $value = Message::rawParam( $value );
+                               break;
+                       case 'msg':
+                               $value = $this->msg( $value )->text();
+                               break;
+                       case 'msg-content':
+                               $value = $this->msg( $value )->inContentLanguage()->text();
+                               break;
+                       case 'number':
+                               $value = Message::numParam( $value );
+                               break;
+                       case 'user':
+                               $user = User::newFromName( $value );
+                               $value = $user->getName();
+                               break;
+                       case 'user-link':
+                               $this->setShowUserToolLinks( false );
+
+                               $user = User::newFromName( $value );
+                               $value = Message::rawParam( $this->makeUserLink( $user ) );
+
+                               $this->setShowUserToolLinks( $saveLinkFlood );
+                               break;
+                       case 'title':
+                               $title = Title::newFromText( $value );
+                               $value = $title->getPrefixedText();
+                               break;
+                       case 'title-link':
+                               $title = Title::newFromText( $value );
+                               $value = Message::rawParam( $this->makePageLink( $title ) );
+                               break;
+                       case 'plain':
+                               // Plain text, nothing to do
+                       default:
+                               // Catch other types and use the old behavior (return as-is)
+               }
+
+               return $value;
+       }
+
        /**
         * Helper to make a link to the page, taking the plaintext
         * value in consideration.
@@ -570,6 +642,16 @@ class LogFormatter {
                return array();
        }
 
+       /**
+        * @return Output of getMessageParameters() for testing
+        */
+       public function getMessageParametersForTesting() {
+               // This function was added because getMessageParameters() is
+               // protected and a change from protected to public caused
+               // problems with extensions
+               return $this->getMessageParameters();
+       }
+
 }
 
 /**

diff --git a/tests/phpunit/includes/logging/LogFormatterTest.php b/tests/phpunit/includes/logging/LogFormatterTest.php
new file mode 100755 (executable)
index 0000000..d6cbbee
--- /dev/null
+++ b/tests/phpunit/includes/logging/LogFormatterTest.php
@@ -0,0 +1,200 @@
+<?php
+/**
+ * @group Database
+ */
+class LogFormatterTest extends MediaWikiLangTestCase {
+
+       /**
+        * @var User
+        */
+       protected $user;
+
+       /**
+        * @var Title
+        */
+       protected $title;
+
+       /**
+        * @var RequestContext
+        */
+       protected $context;
+
+       protected function setUp() {
+               parent::setUp();
+
+               global $wgLang;
+
+               $this->setMwGlobals( array(
+                       'wgLogTypes' => array( 'phpunit' ),
+                       'wgLogActionsHandlers' => array( 'phpunit/test' => 'LogFormatter',
+                               'phpunit/param' => 'LogFormatter' ),
+                       'wgUser' => User::newFromName( 'Testuser' ),
+                       'wgExtensionMessagesFiles' => array( 'LogTests' => __DIR__.'/LogTests.i18n.php' ),
+               ) );
+
+               $wgLang->getLocalisationCache()->recache( $wgLang->getCode() );
+
+               $this->user = User::newFromName( 'Testuser' );
+               $this->title = Title::newMainPage();
+
+               $this->context = new RequestContext();
+               $this->context->setUser( $this->user );
+               $this->context->setTitle( $this->title );
+               $this->context->setLanguage( $wgLang );
+       }
+
+       public function newLogEntry( $action, $params ) {
+               $logEntry = new ManualLogEntry( 'phpunit', $action );
+               $logEntry->setPerformer( $this->user );
+               $logEntry->setTarget( $this->title );
+               $logEntry->setComment( 'A very good reason' );
+
+               $logEntry->setParameters( $params );
+
+               return $logEntry;
+       }
+
+       public function testNormalLogParams() {
+               $entry = $this->newLogEntry( 'test', array() );
+               $formatter = LogFormatter::newFromEntry( $entry );
+               $formatter->setContext( $this->context );
+
+               $formatter->setShowUserToolLinks( false );
+               $paramsWithoutTools = $formatter->getMessageParametersForTesting();
+               unset( $formatter->parsedParameters );
+
+               $formatter->setShowUserToolLinks( true );
+               $paramsWithTools = $formatter->getMessageParametersForTesting();
+
+               $userLink = Linker::userLink(
+                       $this->user->getId(),
+                       $this->user->getName()
+               );
+
+               $userTools = Linker::userToolLinksRedContribs(
+                       $this->user->getId(),
+                       $this->user->getName(),
+                       $this->user->getEditCount()
+               );
+
+               $titleLink = Linker::link( $this->title, null, array(), array() );
+
+               // $paramsWithoutTools and $paramsWithTools should be only different
+               // in index 0
+               $this->assertEquals( $paramsWithoutTools[1], $paramsWithTools[1] );
+               $this->assertEquals( $paramsWithoutTools[2], $paramsWithTools[2] );
+
+               $this->assertEquals( $userLink, $paramsWithoutTools[0]['raw'] );
+               $this->assertEquals( $userLink . $userTools, $paramsWithTools[0]['raw'] );
+
+               $this->assertEquals( $this->user->getName(), $paramsWithoutTools[1] );
+
+               $this->assertEquals( $titleLink, $paramsWithoutTools[2]['raw'] );
+       }
+
+       public function testLogParamsTypeRaw() {
+               $params = array( '4:raw:raw' => Linker::link( $this->title, null, array(), array() ) );
+               $expected = Linker::link( $this->title, null, array(), array() );
+
+               $entry = $this->newLogEntry( 'param', $params );
+               $formatter = LogFormatter::newFromEntry( $entry );
+               $formatter->setContext( $this->context );
+
+               $logParam = $formatter->getActionText();
+
+               $this->assertEquals( $expected, $logParam );
+       }
+
+       public function testLogParamsTypeMsg() {
+               $params = array( '4:msg:msg' => 'log-description-phpunit' );
+               $expected = wfMessage( 'log-description-phpunit' )->text();
+
+               $entry = $this->newLogEntry( 'param', $params );
+               $formatter = LogFormatter::newFromEntry( $entry );
+               $formatter->setContext( $this->context );
+
+               $logParam = $formatter->getActionText();
+
+               $this->assertEquals( $expected, $logParam );
+       }
+
+       public function testLogParamsTypeMsgContent() {
+               $params = array( '4:msg-content:msgContent' => 'log-description-phpunit' );
+               $expected = wfMessage( 'log-description-phpunit' )->inContentLanguage()->text();
+
+               $entry = $this->newLogEntry( 'param', $params );
+               $formatter = LogFormatter::newFromEntry( $entry );
+               $formatter->setContext( $this->context );
+
+               $logParam = $formatter->getActionText();
+
+               $this->assertEquals( $expected, $logParam );
+       }
+
+       public function testLogParamsTypeNumber() {
+               global $wgLang;
+
+               $params = array( '4:number:number' => 123456789 );
+               $expected = $wgLang->formatNum( 123456789 );
+
+               $entry = $this->newLogEntry( 'param', $params );
+               $formatter = LogFormatter::newFromEntry( $entry );
+               $formatter->setContext( $this->context );
+
+               $logParam = $formatter->getActionText();
+
+               $this->assertEquals( $expected, $logParam );
+       }
+
+       public function testLogParamsTypeUserLink() {
+               $params = array( '4:user-link:userLink' => $this->user->getName() );
+               $expected = Linker::userLink(
+                       $this->user->getId(),
+                       $this->user->getName()
+               );
+
+               $entry = $this->newLogEntry( 'param', $params );
+               $formatter = LogFormatter::newFromEntry( $entry );
+               $formatter->setContext( $this->context );
+
+               $logParam = $formatter->getActionText();
+
+               $this->assertEquals( $expected, $logParam );
+       }
+
+       public function testLogParamsTypeTitleLink() {
+               $params = array( '4:title-link:titleLink' => $this->title->getText() );
+               $expected = Linker::link( $this->title, null, array(), array() );
+
+               $entry = $this->newLogEntry( 'param', $params );
+               $formatter = LogFormatter::newFromEntry( $entry );
+               $formatter->setContext( $this->context );
+
+               $logParam = $formatter->getActionText();
+
+               $this->assertEquals( $expected, $logParam );
+       }
+
+       public function testLogParamsTypePlain() {
+               $params = array( '4:plain:plain' => 'Some plain text' );
+               $expected = 'Some plain text';
+
+               $entry = $this->newLogEntry( 'param', $params );
+               $formatter = LogFormatter::newFromEntry( $entry );
+               $formatter->setContext( $this->context );
+
+               $logParam = $formatter->getActionText();
+
+               $this->assertEquals( $expected, $logParam );
+       }
+
+       public function testLogComment() {
+               $entry = $this->newLogEntry( 'test', array() );
+               $formatter = LogFormatter::newFromEntry( $entry );
+               $formatter->setContext( $this->context );
+
+               $comment = ltrim( Linker::commentBlock( $entry->getComment() ) );
+
+               $this->assertEquals( $comment, $formatter->getComment() );
+       }
+}

diff --git a/tests/phpunit/includes/logging/LogTests.i18n.php b/tests/phpunit/includes/logging/LogTests.i18n.php
new file mode 100755 (executable)
index 0000000..8a0a421
--- /dev/null
+++ b/tests/phpunit/includes/logging/LogTests.i18n.php
@@ -0,0 +1,15 @@
+<?php
+/**
+ * Internationalisation file for log tests.
+ *
+ * @file
+ */
+
+$messages = array();
+
+$messages['en'] = array(
+       'log-name-phpunit'        => 'PHPUnit-log',
+       'log-description-phpunit' => 'Log for PHPUnit-tests',
+       'logentry-phpunit-test'   => '$1 {{GENDER:$2|tests}} with page $3',
+       'logentry-phpunit-param'  => '$4',
+);
\ No newline at end of file