Refactor RCFeed configuration (backwards compatible)
authorTimo Tijhof <krinklemail@gmail.com>
Fri, 6 Jan 2017 01:27:43 +0000 (17:27 -0800)
committerAaron Schulz <aschulz@wikimedia.org>
Wed, 1 Feb 2017 04:23:38 +0000 (04:23 +0000)
Previously:
* Engines had to be registered in $wgRCEngines.
* The RCFeedEngine classes took no constructor arguments and
  were expected to send whatever text is previously formatted
  without any information about it. This generic design was
  flexible in allowing one to use any formatter with any engine
  with minimal configuration and no need for additional classes.
* Each feed configured their destination by setting a 'uri'
  option that encodes the name of the engine in PHP as the uri
  scheme. Other uri components had to be used for any other
  parameters to the engine (host, port, path). While fairly
  limited, it was sufficient for the default engines in core.

Changes:
* Allow feed classes to be directly associated with a feed in $wgRCFeeds
  via a new 'class' option - without the indirection of 'uri' and
  $wgRCEngines. All options are passed to the given class constructor.
  This matches the design used elsewhere in MediaWiki. (ObjectCache,
  FileRepo, FileBackend, JobQueue, LBFactory, etc.)

  This means we no longer enforce a 1:1 mapping of internet protocols
  to a specific feed engine, and it allows settings to be passed
  without being encoded as a URI neccecarily.

  Main use case for this refactor is EventBus (see I7edc4d57fa),

  Interestingly, this matches the (then incorrect) documentation
  written for $wgRCFeeds in 2961884b43 (which mentions an 'engine'
  property that would do the same thing).

* Move the default 'omit' filters and unrestricted 'formatter' handling
  to a new FormattedRCFeed class, which remains the default.

* Deprecate RecentChange::getEngine() in favour of RCFeed::factory().

* Document wgRCEngines as "@since 1.22". Follows 2961884b43ffc71cb6af.

Change-Id: I8be497c623c5d928762e3d3406a388f4d91add9a

autoload.php
includes/DefaultSettings.php
includes/changes/RecentChange.php
includes/rcfeed/FormattedRCFeed.php [new file with mode: 0644]
includes/rcfeed/RCFeed.php [new file with mode: 0644]
includes/rcfeed/RCFeedEngine.php
includes/rcfeed/RedisPubSubFeedEngine.php
includes/rcfeed/UDPRCFeedEngine.php
tests/phpunit/includes/rcfeed/RCFeedIntegrationTest.php

index a6840b4..31c8846 100644 (file)
@@ -506,6 +506,7 @@ $wgAutoloadLocalClasses = [
        'FormSpecialPage' => __DIR__ . '/includes/specialpage/FormSpecialPage.php',
        'FormatJson' => __DIR__ . '/includes/json/FormatJson.php',
        'FormatMetadata' => __DIR__ . '/includes/media/FormatMetadata.php',
+       'FormattedRCFeed' => __DIR__ . '/includes/rcfeed/FormattedRCFeed.php',
        'FormlessAction' => __DIR__ . '/includes/actions/FormlessAction.php',
        'GIFHandler' => __DIR__ . '/includes/media/GIF.php',
        'GIFMetadataExtractor' => __DIR__ . '/includes/media/GIFMetadataExtractor.php',
@@ -1146,6 +1147,7 @@ $wgAutoloadLocalClasses = [
        'RCCacheEntry' => __DIR__ . '/includes/changes/RCCacheEntry.php',
        'RCCacheEntryFactory' => __DIR__ . '/includes/changes/RCCacheEntryFactory.php',
        'RCDatabaseLogEntry' => __DIR__ . '/includes/logging/LogEntry.php',
+       'RCFeed' => __DIR__ . '/includes/rcfeed/RCFeed.php',
        'RCFeedEngine' => __DIR__ . '/includes/rcfeed/RCFeedEngine.php',
        'RCFeedFormatter' => __DIR__ . '/includes/rcfeed/RCFeedFormatter.php',
        'RESTBagOStuff' => __DIR__ . '/includes/libs/objectcache/RESTBagOStuff.php',
index a7cbd96..c7c7fb7 100644 (file)
@@ -6656,51 +6656,64 @@ $wgRCLinkLimits = [ 50, 100, 250, 500 ];
 $wgRCLinkDays = [ 1, 3, 7, 14, 30 ];
 
 /**
- * Destinations to which notifications about recent changes
- * should be sent.
- *
- * As of MediaWiki 1.22, there are 2 supported 'engine' parameter option in core:
- *   * 'UDPRCFeedEngine', which is used to send recent changes over UDP to the
- *      specified server.
- *   * 'RedisPubSubFeedEngine', which is used to send recent changes to Redis.
- *
- * The common options are:
- *   * 'uri' -- the address to which the notices are to be sent.
- *   * 'formatter' -- the class name (implementing RCFeedFormatter) which will
- *     produce the text to send. This can also be an object of the class.
- *   * 'omit_bots' -- whether the bot edits should be in the feed
- *   * 'omit_anon' -- whether anonymous edits should be in the feed
- *   * 'omit_user' -- whether edits by registered users should be in the feed
- *   * 'omit_minor' -- whether minor edits should be in the feed
- *   * 'omit_patrolled' -- whether patrolled edits should be in the feed
- *
- *  The IRC-specific options are:
- *   * 'add_interwiki_prefix' -- whether the titles should be prefixed with
- *     the first entry in the $wgLocalInterwikis array (or the value of
- *     $wgLocalInterwiki, if set)
- *
- *  The JSON-specific options are:
- *   * 'channel' -- if set, the 'channel' parameter is also set in JSON values.
+ * Configuration for feeds to which notifications about recent changes will be sent.
+ *
+ * The following feed classes are available by default:
+ * - 'UDPRCFeedEngine' - sends recent changes over UDP to the specified server.
+ * - 'RedisPubSubFeedEngine' - send recent changes to Redis.
+ *
+ * Only 'class' or 'uri' is required. If 'uri' is set instead of 'class', then
+ * RecentChange::getEngine() is used to determine the class. All options are
+ * passed to the constructor.
+ *
+ * Common options:
+ * - 'class' -- The class to use for this feed (must implement RCFeed).
+ * - 'omit_bots' -- Exclude bot edits from the feed. (default: false)
+ * - 'omit_anon' -- Exclude anonymous edits from the feed. (default: false)
+ * - 'omit_user' -- Exclude edits by registered users from the feed. (default: false)
+ * - 'omit_minor' -- Exclude minor edits from the feed. (default: false)
+ * - 'omit_patrolled' -- Exclude patrolled edits from the feed. (default: false)
+ *
+ * FormattedRCFeed-specific options:
+ * - 'uri' -- [required] The address to which the messages are sent.
+ *   The uri scheme of this string will be looked up in $wgRCEngines
+ *   to determine which RCFeedEngine class to use.
+ * - 'formatter' -- [required] The class (implementing RCFeedFormatter) which will
+ *   produce the text to send. This can also be an object of the class.
+ *   Formatters available by default: JSONRCFeedFormatter, XMLRCFeedFormatter,
+ *   IRCColourfulRCFeedFormatter.
+ *
+ * IRCColourfulRCFeedFormatter-specific options:
+ * - 'add_interwiki_prefix' -- whether the titles should be prefixed with
+ *   the first entry in the $wgLocalInterwikis array (or the value of
+ *   $wgLocalInterwiki, if set)
+ *
+ * JSONRCFeedFormatter-specific options:
+ * - 'channel' -- if set, the 'channel' parameter is also set in JSON values.
  *
  * @example $wgRCFeeds['example'] = [
+ *             'uri' => 'udp://localhost:1336',
  *             'formatter' => 'JSONRCFeedFormatter',
- *             'uri' => "udp://localhost:1336",
  *             'add_interwiki_prefix' => false,
  *             'omit_bots' => true,
  *     ];
- * @example $wgRCFeeds['exampleirc'] = [
+ * @example $wgRCFeeds['example'] = [
+ *             'uri' => 'udp://localhost:1338',
  *             'formatter' => 'IRCColourfulRCFeedFormatter',
- *             'uri' => "udp://localhost:1338",
  *             'add_interwiki_prefix' => false,
  *             'omit_bots' => true,
  *     ];
+ * @example $wgRCFeeds['example'] = [
+ *      'class' => 'ExampleRCFeed',
+ *     ];
  * @since 1.22
  */
 $wgRCFeeds = [];
 
 /**
- * Used by RecentChange::getEngine to find the correct engine to use for a given URI scheme.
- * Keys are scheme names, values are names of engine classes.
+ * Used by RecentChange::getEngine to find the correct engine for a given URI scheme.
+ * Keys are scheme names, values are names of FormattedRCFeed sub classes.
+ * @since 1.22
  */
 $wgRCEngines = [
        'redis' => 'RedisPubSubFeedEngine',
index 13a5fc7..b651f86 100644 (file)
@@ -389,8 +389,8 @@ class RecentChange {
 
                $performer = $this->getPerformer();
 
-               foreach ( $feeds as $feed ) {
-                       $feed += [
+               foreach ( $feeds as $params ) {
+                       $params += [
                                'omit_bots' => false,
                                'omit_anon' => false,
                                'omit_user' => false,
@@ -399,58 +399,44 @@ class RecentChange {
                        ];
 
                        if (
-                               ( $feed['omit_bots'] && $this->mAttribs['rc_bot'] ) ||
-                               ( $feed['omit_anon'] && $performer->isAnon() ) ||
-                               ( $feed['omit_user'] && !$performer->isAnon() ) ||
-                               ( $feed['omit_minor'] && $this->mAttribs['rc_minor'] ) ||
-                               ( $feed['omit_patrolled'] && $this->mAttribs['rc_patrolled'] ) ||
+                               ( $params['omit_bots'] && $this->mAttribs['rc_bot'] ) ||
+                               ( $params['omit_anon'] && $performer->isAnon() ) ||
+                               ( $params['omit_user'] && !$performer->isAnon() ) ||
+                               ( $params['omit_minor'] && $this->mAttribs['rc_minor'] ) ||
+                               ( $params['omit_patrolled'] && $this->mAttribs['rc_patrolled'] ) ||
                                $this->mAttribs['rc_type'] == RC_EXTERNAL
                        ) {
                                continue;
                        }
 
-                       $engine = self::getEngine( $feed['uri'] );
-
                        if ( isset( $this->mExtra['actionCommentIRC'] ) ) {
                                $actionComment = $this->mExtra['actionCommentIRC'];
                        } else {
                                $actionComment = null;
                        }
 
-                       /** @var $formatter RCFeedFormatter */
-                       $formatter = is_object( $feed['formatter'] ) ? $feed['formatter'] : new $feed['formatter']();
-                       $line = $formatter->getLine( $feed, $this, $actionComment );
-                       if ( !$line ) {
-                               // T109544
-                               // If a feed formatter returns null, this will otherwise cause an
-                               // error in at least RedisPubSubFeedEngine.
-                               // Not sure where/how this should best be handled.
-                               continue;
-                       }
-
-                       $engine->send( $feed, $line );
+                       $feed = RCFeed::factory( $params );
+                       $feed->notify( $this, $actionComment );
                }
        }
 
        /**
-        * Gets the stream engine object for a given URI from $wgRCEngines
-        *
+        * @since 1.22
+        * @deprecated since 1.29 Use RCFeed::factory() instead
         * @param string $uri URI to get the engine object for
-        * @throws MWException
         * @return RCFeedEngine The engine object
+        * @throws MWException
         */
        public static function getEngine( $uri ) {
+               // TODO: Merge into RCFeed::factory().
                global $wgRCEngines;
-
                $scheme = parse_url( $uri, PHP_URL_SCHEME );
                if ( !$scheme ) {
-                       throw new MWException( __FUNCTION__ . ": Invalid stream logger URI: '$uri'" );
+                       throw new MWException( "Invalid RCFeed uri: '$uri'" );
                }
-
                if ( !isset( $wgRCEngines[$scheme] ) ) {
-                       throw new MWException( __FUNCTION__ . ": Unknown stream logger URI scheme: $scheme" );
+                       throw new MWException( "Unknown RCFeedEngine scheme: '$scheme'" );
                }
-
                if ( defined( 'MW_PHPUNIT_TEST' ) && is_object( $wgRCEngines[$scheme] ) ) {
                        return $wgRCEngines[$scheme];
                }
diff --git a/includes/rcfeed/FormattedRCFeed.php b/includes/rcfeed/FormattedRCFeed.php
new file mode 100644 (file)
index 0000000..d841681
--- /dev/null
@@ -0,0 +1,68 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+/**
+ * Base class for RC feed engines that send messages in a freely configurable
+ * format to a uri-addressed engine set in $wgRCEngines.
+ * @since 1.29
+ */
+abstract class FormattedRCFeed extends RCFeed {
+       private $params;
+
+       /**
+        * @param array $params
+        *  - 'uri'
+        *  - 'formatter'
+        * @see $wgRCFeeds
+        */
+       public function __construct( array $params ) {
+               $this->params = $params;
+       }
+
+       /**
+        * Send some text to the specified feed.
+        *
+        * @param array $feed The feed, as configured in an associative array
+        * @param string $line The text to send
+        * @return bool Success
+        */
+       abstract public function send( array $feed, $line );
+
+       /**
+        * @param RecentChange $rc
+        * @param string|null $actionComment
+        * @return bool Success
+        */
+       public function notify( RecentChange $rc, $actionComment = null ) {
+               $params = $this->params;
+               /** @var $formatter RCFeedFormatter */
+               $formatter = is_object( $params['formatter'] ) ? $params['formatter'] : new $params['formatter'];
+
+               $line = $formatter->getLine( $params, $rc, $actionComment );
+               if ( !$line ) {
+                       // @codeCoverageIgnoreStart
+                       // T109544 - If a feed formatter returns null, this will otherwise cause an
+                       // error in at least RedisPubSubFeedEngine. Not sure best to handle this.
+                       return;
+                       // @codeCoverageIgnoreEnd
+               }
+               return $this->send( $params, $line );
+       }
+}
diff --git a/includes/rcfeed/RCFeed.php b/includes/rcfeed/RCFeed.php
new file mode 100644 (file)
index 0000000..7e9ce60
--- /dev/null
@@ -0,0 +1,59 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+/**
+ * @see $wgRCFeeds
+ * @since 1.29
+ */
+abstract class RCFeed {
+       /**
+        * @param array $params
+        */
+       public function __construct( array $params = [] ) {
+       }
+
+       /**
+        * Dispatch the recent changes notification.
+        *
+        * @param RecentChange $rc
+        * @param string|null $actionComment
+        * @return bool Success
+        */
+       abstract public function notify( RecentChange $rc, $actionComment = null );
+
+       /**
+        * @param array $params
+        * @return RCFeed
+        * @throws Exception
+        */
+       final public static function factory( array $params ) {
+               if ( !isset( $params['class'] ) ) {
+                       if ( !isset( $params['uri'] ) ) {
+                               throw new Exception( "RCFeeds must have a 'class' or 'uri' set." );
+                       }
+                       return RecentChange::getEngine( $params['uri'] );
+               }
+               $class = $params['class'];
+               if ( !class_exists( $class ) ) {
+                       throw new Exception( "Unknown class '$class'." );
+               }
+               return new $class( $params );
+       }
+}
index 0b0cd86..49436fa 100644 (file)
@@ -1,5 +1,4 @@
 <?php
-
 /**
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  */
 
 /**
- * Interface for RC feed engines, which send formatted notifications
- *
+ * Backward-compatibility alias.
  * @since 1.22
+ * @deprecated since 1.29 Use FormattedRCFeed instead
  */
-interface RCFeedEngine {
-       /**
-        * Sends some text to the specified live feed.
-        *
-        * @see IRCColourfulRCFeedFormatter::cleanupForIRC
-        * @param array $feed The feed, as configured in an associative array
-        * @param string $line The text to send
-        * @return bool Success
-        */
-       public function send( array $feed, $line );
+abstract class RCFeedEngine extends FormattedRCFeed {
 }
index c10e959..4c011be 100644 (file)
@@ -1,5 +1,4 @@
 <?php
-
 /**
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -20,7 +19,7 @@
  */
 
 /**
- * Emit a recent change notification via Redis Pub/Sub
+ * Send recent change notifications via Redis Pub/Sub
  *
  * If the feed URI contains a path component, it will be used to generate a
  * channel name by stripping the leading slash and replacing any remaining
  *
  * @since 1.22
  */
-class RedisPubSubFeedEngine implements RCFeedEngine {
+class RedisPubSubFeedEngine extends RCFeedEngine {
 
        /**
-        * @see RCFeedEngine::send
+        * @see FormattedRCFeed::send
         */
        public function send( array $feed, $line ) {
                $parsed = wfParseUrl( $feed['uri'] );
index 9afae66..61ced5f 100644 (file)
@@ -1,5 +1,4 @@
 <?php
-
 /**
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  */
 
 /**
- * Sends the notification to the specified host in a UDP packet.
+ * Send recent change notifications in a UDP packet.
  * @since 1.22
  */
-
-class UDPRCFeedEngine implements RCFeedEngine {
+class UDPRCFeedEngine extends RCFeedEngine {
        /**
         * @see RCFeedEngine::send
         */
index 97ea23c..e3ea139 100644 (file)
@@ -17,7 +17,9 @@ class RCFeedIntegrationTest extends MediaWikiTestCase {
        /**
         * @covers RecentChange::notifyRCFeeds
         * @covers RecentChange::getEngine
-        * @covers RCFeedEngine
+        * @covers RCFeed::factory
+        * @covers FormattedRCFeed::__construct
+        * @covers FormattedRCFeed::notify
         * @covers JSONRCFeedFormatter::formatArray
         * @covers MachineReadableRCFeedFormatter::getLine
         */