Merge "Use isDisabled() instead of isBlank() in getGrantName in User.php"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Tue, 29 Nov 2016 23:03:55 +0000 (23:03 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Tue, 29 Nov 2016 23:03:55 +0000 (23:03 +0000)
autoload.php
includes/debug/logger/monolog/LogstashFormatter.php [new file with mode: 0644]
includes/debug/logger/monolog/WikiProcessor.php
includes/specials/SpecialListgrants.php
includes/user/User.php
languages/i18n/en.json
languages/i18n/qqq.json
resources/src/mediawiki.widgets/mw.widgets.SearchInputWidget.js
resources/src/mediawiki.widgets/mw.widgets.TitleWidget.js
tests/phpunit/includes/debug/logger/monolog/LogstashFormatterTest.php [new file with mode: 0644]

index 30ef985..f0bbe92 100644 (file)
@@ -875,6 +875,7 @@ $wgAutoloadLocalClasses = [
        'MediaWiki\\Logger\\Monolog\\LegacyFormatter' => __DIR__ . '/includes/debug/logger/monolog/LegacyFormatter.php',
        'MediaWiki\\Logger\\Monolog\\LegacyHandler' => __DIR__ . '/includes/debug/logger/monolog/LegacyHandler.php',
        'MediaWiki\\Logger\\Monolog\\LineFormatter' => __DIR__ . '/includes/debug/logger/monolog/LineFormatter.php',
+       'MediaWiki\\Logger\\Monolog\\LogstashFormatter' => __DIR__ . '/includes/debug/logger/monolog/LogstashFormatter.php',
        'MediaWiki\\Logger\\Monolog\\SyslogHandler' => __DIR__ . '/includes/debug/logger/monolog/SyslogHandler.php',
        'MediaWiki\\Logger\\Monolog\\WikiProcessor' => __DIR__ . '/includes/debug/logger/monolog/WikiProcessor.php',
        'MediaWiki\\Logger\\NullSpi' => __DIR__ . '/includes/debug/logger/NullSpi.php',
diff --git a/includes/debug/logger/monolog/LogstashFormatter.php b/includes/debug/logger/monolog/LogstashFormatter.php
new file mode 100644 (file)
index 0000000..553cbf6
--- /dev/null
@@ -0,0 +1,83 @@
+<?php
+
+namespace MediaWiki\Logger\Monolog;
+
+/**
+ * LogstashFormatter squashes the base message array and the context and extras subarrays into one.
+ * This can result in unfortunately named context fields overwriting other data (T145133).
+ * This class modifies the standard LogstashFormatter to rename such fields and flag the message.
+ *
+ * Compatible with Monolog 1.x only.
+ *
+ * @since 1.29
+ */
+class LogstashFormatter extends \Monolog\Formatter\LogstashFormatter {
+       /** @var array Keys which should not be used in log context */
+       protected $reservedKeys = [
+               // from LogstashFormatter
+               'message', 'channel', 'level', 'type',
+               // from WebProcessor
+               'url', 'ip', 'http_method', 'server', 'referrer',
+               // from WikiProcessor
+               'host', 'wiki', 'reqId', 'mwversion',
+               // from config magic
+               'normalized_message',
+       ];
+
+       /**
+        * Prevent key conflicts
+        * @param array $record
+        * @return array
+        */
+       protected function formatV0( array $record ) {
+               if ( $this->contextPrefix ) {
+                       return parent::formatV0( $record );
+               }
+
+               $context = !empty( $record['context'] ) ? $record['context'] : [];
+               $record['context'] = [];
+               $formatted = parent::formatV0( $record );
+
+               $formatted['@fields'] = $this->fixKeyConflicts( $formatted['@fields'], $context );
+               return $formatted;
+       }
+
+       /**
+        * Prevent key conflicts
+        * @param array $record
+        * @return array
+        */
+       protected function formatV1( array $record ) {
+               if ( $this->contextPrefix ) {
+                       return parent::formatV1( $record );
+               }
+
+               $context = !empty( $record['context'] ) ? $record['context'] : [];
+               $record['context'] = [];
+               $formatted = parent::formatV1( $record );
+
+               $formatted = $this->fixKeyConflicts( $formatted, $context );
+               return $formatted;
+       }
+
+       /**
+        * Check whether some context field would overwrite another message key. If so, rename
+        * and flag.
+        * @param array $fields Fields to be sent to logstash
+        * @param array $context Copy of the original $record['context']
+        * @return array Updated version of $fields
+        */
+       protected function fixKeyConflicts( array $fields, array $context ) {
+               foreach ( $context as $key => $val ) {
+                       if (
+                               in_array( $key, $this->reservedKeys, true ) &&
+                               isset( $fields[$key] ) && $fields[$key] !== $val
+                       ) {
+                               $fields['logstash_formatter_key_conflict'][] = $key;
+                               $key = 'c_' . $key;
+                       }
+                       $fields[$key] = $val;
+               }
+               return $fields;
+       }
+}
index 81e1e14..ad939a0 100644 (file)
@@ -29,17 +29,6 @@ namespace MediaWiki\Logger\Monolog;
  * @copyright © 2013 Bryan Davis and Wikimedia Foundation.
  */
 class WikiProcessor {
-       /** @var array Keys which should not be used in log context */
-       protected $reservedKeys = [
-               // from monolog:src/Monolog/Formatter/LogstashFormatter.php#L71-L88
-               'message', 'channel', 'level', 'type',
-               // from WebProcessor
-               'url', 'ip', 'http_method', 'server', 'referrer',
-               // from WikiProcessor
-               'host', 'wiki', 'reqId', 'mwversion',
-               // from config magic
-               'normalized_message',
-       ];
 
        /**
         * @param array $record
@@ -47,15 +36,6 @@ class WikiProcessor {
         */
        public function __invoke( array $record ) {
                global $wgVersion;
-
-               // some log aggregators such as Logstash will merge the log context into the main
-               // metadata and end up overwriting the data coming from processors
-               foreach ( $this->reservedKeys as $key ) {
-                       if ( isset( $record['context'][$key] ) ) {
-                               wfLogWarning( __METHOD__ . ": '$key' key overwritten in log context." );
-                       }
-               }
-
                $record['extra'] = array_merge(
                        $record['extra'],
                        [
@@ -67,4 +47,5 @@ class WikiProcessor {
                );
                return $record;
        }
+
 }
index 39c8ae8..2c92410 100644 (file)
@@ -71,8 +71,14 @@ class SpecialListGrants extends SpecialPage {
 
                        $id = \Sanitizer::escapeId( $grant );
                        $out->addHTML( \Html::rawElement( 'tr', [ 'id' => $id ],
-                               "<td>" . $this->msg( "grant-$grant" )->escaped() . "</td>" .
-                               "<td>" . $grantCellHtml . '</td>'
+                               "<td>" .
+                               $this->msg(
+                                       "listgrants-grant-display",
+                                       \User::getGrantName( $grant ),
+                                       "<span class='mw-listgrants-grant-name'>" . $id . "</span>"
+                               )->parse() .
+                               "</td>" .
+                               "<td>" . $grantCellHtml . "</td>"
                        ) );
                }
 
index 4950e27..f07db0e 100644 (file)
@@ -5076,6 +5076,19 @@ class User implements IDBAccessObject {
                return $msg->isDisabled() ? $right : $msg->text();
        }
 
+       /**
+        * Get the name of a given grant
+        *
+        * @since 1.29
+        * @param string $grant Grant to query
+        * @return string Localized name of the grant
+        */
+       public static function getGrantName( $grant ) {
+               $key = "grant-$grant";
+               $msg = wfMessage( $key );
+               return $msg->isDisabled() ? $grant : $msg->text();
+       }
+
        /**
         * Make a new-style password hash
         *
index 3a8a7ae..a49f95c 100644 (file)
        "listgrants-summary": "The following is a list of grants with their associated access to user rights. Users can authorize applications to use their account, but with limited permissions based on the grants the user gave to the application. An application acting on behalf of a user cannot actually use rights that the user does not have however.\nThere may be [[{{MediaWiki:Listgrouprights-helppage}}|additional information]] about individual rights.",
        "listgrants-grant": "Grant",
        "listgrants-rights": "Rights",
+       "listgrants-grant-display": "$1 <code>($2)</code>",
        "trackingcategories": "Tracking categories",
        "trackingcategories-summary": "This page lists tracking categories which are automatically populated by the MediaWiki software. Their names can be changed by altering the relevant system messages in the {{ns:8}} namespace.",
        "trackingcategories-msg": "Tracking category",
index d175c86..84bca9a 100644 (file)
        "listgrants-summary": "Explanatory text shown at the top of the grant/rights mapping table.\n\nRefers to {{msg-mw|Listgrouprights-helppage}}.",
        "listgrants-grant": "Used as table header for the grant/rights mapping table.\n{{Identical|Grant}}",
        "listgrants-rights": "Used as table header for the grant/rights mapping table.\n{{Identical|Right}}",
+       "listgrants-grant-display": "{{optional}}\nUsed to display the code name of a grant next to the grant. Parameters:\n* $1 - the text from the \"grant-...\" messages, i.e. {{msg-mw|Grant-highvolume}}\n* $2 - the codename of this grant",
        "trackingcategories": "[[Special:TrackingCategories]] page implementing list of Tracking categories [[mw:Special:MyLanguage/Help:Tracking categories|tracking category]].\n{{Identical|Tracking category}}",
        "trackingcategories-summary": "Description for [[Special:TrackingCategories]] page [[mw:Help:Tracking categories|tracking category]]",
        "trackingcategories-msg": "Header for the message column of the table on [[Special:TrackingCategories]]. This column lists the mediawiki message that controls the tracking category in question.\n{{Identical|Tracking category}}",
index 39bee7c..2ac75c5 100755 (executable)
@@ -78,7 +78,7 @@
         * @inheritdoc mw.widgets.TitleWidget
         */
        mw.widgets.SearchInputWidget.prototype.getSuggestionsPromise = function () {
-               var api = new mw.Api(),
+               var api = this.getApi(),
                        promise,
                        self = this;
 
index e1e50ea..0e5e0c5 100644 (file)
@@ -6,16 +6,6 @@
  */
 ( function ( $, mw ) {
 
-       var interwikiPrefixesPromise = new mw.Api().get( {
-               action: 'query',
-               meta: 'siteinfo',
-               siprop: 'interwikimap'
-       } ).then( function ( data ) {
-               return $.map( data.query.interwikimap, function ( interwiki ) {
-                       return interwiki.prefix;
-               } );
-       } );
-
        /**
         * Mixin for title widgets
         *
@@ -36,6 +26,7 @@
         * @cfg {boolean} [validateTitle=true] Whether the input must be a valid title (if set to true,
         *  the widget will marks itself red for invalid inputs, including an empty query).
         * @cfg {Object} [cache] Result cache which implements a 'set' method, taking keyed values as an argument
+        * @cfg {mw.Api} [api] API object to use, creates a default mw.Api instance if not specified
         */
        mw.widgets.TitleWidget = function MwWidgetsTitleWidget( config ) {
                // Config initialization
@@ -56,6 +47,7 @@
                this.excludeCurrentPage = !!config.excludeCurrentPage;
                this.validateTitle = config.validateTitle !== undefined ? config.validateTitle : true;
                this.cache = config.cache;
+               this.api = config.api || new mw.Api();
 
                // Initialization
                this.$element.addClass( 'mw-widget-titleWidget' );
 
        OO.initClass( mw.widgets.TitleWidget );
 
+       /* Static properties */
+
+       mw.widgets.TitleWidget.static.interwikiPrefixesPromiseCache = {};
+
        /* Methods */
 
        /**
                this.namespace = namespace;
        };
 
+       mw.widgets.TitleWidget.prototype.getInterwikiPrefixesPromise = function () {
+               var api = this.getApi(),
+                       cache = this.constructor.static.interwikiPrefixesPromiseCache,
+                       key = api.defaults.ajax.url;
+               if ( !cache.hasOwnProperty( key ) ) {
+                       cache[ key ] = api.get( {
+                               action: 'query',
+                               meta: 'siteinfo',
+                               siprop: 'interwikimap'
+                       } ).then( function ( data ) {
+                               return $.map( data.query.interwikimap, function ( interwiki ) {
+                                       return interwiki.prefix;
+                               } );
+                       } );
+               }
+               return cache[ key ];
+       };
+
        /**
         * Get a promise which resolves with an API repsonse for suggested
         * links for the current query.
         */
        mw.widgets.TitleWidget.prototype.getSuggestionsPromise = function () {
                var req,
+                       api = this.getApi(),
                        query = this.getQueryValue(),
                        widget = this,
                        promiseAbortObject = { abort: function () {
                        } };
 
                if ( mw.Title.newFromText( query ) ) {
-                       return interwikiPrefixesPromise.then( function ( interwikiPrefixes ) {
+                       return this.getInterwikiPrefixesPromise().then( function ( interwikiPrefixes ) {
                                var params,
                                        interwiki = query.substring( 0, query.indexOf( ':' ) );
                                if (
                                                params.prop.push( 'pageterms' );
                                                params.wbptterms = 'description';
                                        }
-                                       req = new mw.Api().get( params );
+                                       req = api.get( params );
                                        promiseAbortObject.abort = req.abort.bind( req ); // TODO ew
                                        return req.then( function ( ret ) {
                                                if ( ret.query === undefined ) {
-                                                       ret = new mw.Api().get( { action: 'query', titles: query } );
+                                                       ret = api.get( { action: 'query', titles: query } );
                                                        promiseAbortObject.abort = ret.abort.bind( ret );
                                                }
                                                return ret;
                }
        };
 
+       /**
+        * Get the API object for title requests
+        *
+        * @return {mw.Api} MediaWiki API
+        */
+       mw.widgets.TitleWidget.prototype.getApi = function () {
+               return this.api;
+       };
+
        /**
         * Get option widgets from the server response
         *
diff --git a/tests/phpunit/includes/debug/logger/monolog/LogstashFormatterTest.php b/tests/phpunit/includes/debug/logger/monolog/LogstashFormatterTest.php
new file mode 100644 (file)
index 0000000..8086b4b
--- /dev/null
@@ -0,0 +1,55 @@
+<?php
+
+namespace MediaWiki\Logger\Monolog;
+
+class LogstashFormatterTest extends \PHPUnit_Framework_TestCase {
+       /**
+        * @dataProvider provideV1
+        * @param array $record The input record.
+        * @param array $expected Associative array of expected keys and their values.
+        * @param array $notExpected List of keys that should not exist.
+        */
+       public function testV1( array $record, array $expected, array $notExpected ) {
+               $formatter = new LogstashFormatter( 'app', 'system', null, null, LogstashFormatter::V1 );
+               $formatted = json_decode( $formatter->format( $record ), true );
+               foreach ( $expected as $key => $value ) {
+                       $this->assertArrayHasKey( $key, $formatted );
+                       $this->assertSame( $value, $formatted[$key] );
+               }
+               foreach ( $notExpected as $key ) {
+                       $this->assertArrayNotHasKey( $key, $formatted );
+               }
+       }
+
+       public function provideV1() {
+               return [
+                       [
+                               [ 'extra' => [ 'foo' => 1 ], 'context' => [ 'bar' => 2 ] ],
+                               [ 'foo' => 1, 'bar' => 2 ],
+                               [ 'logstash_formatter_key_conflict' ],
+                       ],
+                       [
+                               [ 'extra' => [ 'url' => 1 ], 'context' => [ 'url' => 2 ] ],
+                               [ 'url' => 1, 'c_url' => 2, 'logstash_formatter_key_conflict' => [ 'url' ] ],
+                               [],
+                       ],
+                       [
+                               [ 'channel' => 'x', 'context' => [ 'channel' => 'y' ] ],
+                               [ 'channel' => 'x', 'c_channel' => 'y',
+                                       'logstash_formatter_key_conflict' => [ 'channel' ] ],
+                               [],
+                       ],
+               ];
+       }
+
+       public function testV1WithPrefix() {
+               $formatter = new LogstashFormatter( 'app', 'system', null, 'ctx_', LogstashFormatter::V1 );
+               $record = [ 'extra' => [ 'url' => 1 ], 'context' => [ 'url' => 2 ] ];
+               $formatted = json_decode( $formatter->format( $record ), true );
+               $this->assertArrayHasKey( 'url', $formatted );
+               $this->assertSame( 1, $formatted['url'] );
+               $this->assertArrayHasKey( 'ctx_url', $formatted );
+               $this->assertSame( 2, $formatted['ctx_url'] );
+               $this->assertArrayNotHasKey( 'c_url', $formatted );
+       }
+}