Sbisson has uploaded a new change for review.
https://gerrit.wikimedia.org/r/300032
Change subject: Remove deprecated formatter
......................................................................
Remove deprecated formatter
Followup to I639b9d9906d3ff37021cb9b5ed3cb401354b5bd9
* Remove deprecated formatter
* Log a warning and fail gracefully
when an event type does not support
Echo presentation model.
Bug: T121612
Change-Id: Ic5712c4ce265b6faabce7a4028b4294fe3c73f18
---
M autoload.php
M includes/DataOutputFormatter.php
M includes/controller/NotificationController.php
M includes/formatters/EchoEventFormatter.php
M includes/formatters/EchoFlyoutFormatter.php
M includes/formatters/EchoHtmlDigestEmailFormatter.php
M includes/formatters/EchoHtmlEmailFormatter.php
A includes/formatters/EchoIcon.php
M includes/formatters/EchoModelFormatter.php
D includes/formatters/NotificationFormatter.php
M includes/formatters/SpecialNotificationsFormatter.php
D tests/phpunit/formatters/NotificationFormatterTest.php
12 files changed, 68 insertions(+), 371 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Echo
refs/changes/32/300032/1
diff --git a/autoload.php b/autoload.php
index 8ac6fd9..38420aa 100644
--- a/autoload.php
+++ b/autoload.php
@@ -56,6 +56,7 @@
'EchoHooks' => __DIR__ . '/Hooks.php',
'EchoHtmlDigestEmailFormatter' => __DIR__ .
'/includes/formatters/EchoHtmlDigestEmailFormatter.php',
'EchoHtmlEmailFormatter' => __DIR__ .
'/includes/formatters/EchoHtmlEmailFormatter.php',
+ 'EchoIcon' => __DIR__ . '/includes/formatters/EchoIcon.php',
'EchoIteratorDecorator' => __DIR__ .
'/includes/iterator/IteratorDecorator.php',
'EchoLocalCache' => __DIR__ . '/includes/cache/LocalCache.php',
'EchoMentionPresentationModel' => __DIR__ .
'/includes/formatters/MentionPresentationModel.php',
@@ -66,8 +67,6 @@
'EchoNotification' => __DIR__ . '/includes/model/Notification.php',
'EchoNotificationController' => __DIR__ .
'/includes/controller/NotificationController.php',
'EchoNotificationDeleteJob' => __DIR__ .
'/includes/jobs/NotificationDeleteJob.php',
- 'EchoNotificationFormatter' => __DIR__ .
'/includes/formatters/NotificationFormatter.php',
- 'EchoNotificationFormatterTest' => __DIR__ .
'/tests/phpunit/formatters/NotificationFormatterTest.php',
'EchoNotificationJob' => __DIR__ . '/includes/jobs/NotificationJob.php',
'EchoNotificationMapper' => __DIR__ .
'/includes/mapper/NotificationMapper.php',
'EchoNotificationMapperTest' => __DIR__ .
'/tests/phpunit/mapper/NotificationMapperTest.php',
diff --git a/includes/DataOutputFormatter.php b/includes/DataOutputFormatter.php
index 6a763fe..6869712 100644
--- a/includes/DataOutputFormatter.php
+++ b/includes/DataOutputFormatter.php
@@ -159,15 +159,12 @@
* @return string|bool false if it could not be formatted
*/
protected static function formatNotification( EchoEvent $event, User
$user, $format, $lang ) {
- if ( isset( self::$formatters[$format] )
- &&
EchoEventPresentationModel::supportsPresentationModel( $event->getType() )
- ) {
+ if ( isset( self::$formatters[$format] ) ) {
/** @var EchoEventFormatter $formatter */
$formatter = new self::$formatters[$format]( $user,
$lang );
return $formatter->format( $event );
} else {
- // Legacy b/c
- return EchoNotificationController::formatNotification(
$event, $user, $format );
+ return false;
}
}
diff --git a/includes/controller/NotificationController.php
b/includes/controller/NotificationController.php
index 837dc4e..918a943 100644
--- a/includes/controller/NotificationController.php
+++ b/includes/controller/NotificationController.php
@@ -1,7 +1,5 @@
<?php
-use MediaWiki\Logger\LoggerFactory;
-
/**
* This class represents the controller for notifications
*/
@@ -381,55 +379,6 @@
}
return $notify->getIterator();
- }
-
- /**
- * Formats a notification
- *
- * @param EchoEvent $event The event for a notification.
- * @param User $user The user to format the notification for.
- * @param string $format The format to show the notification in: text,
html, or email
- * @param string $type The type of notification being distributed (e.g.
email, web)
- * @return string|array The formatted notification, or an array of
subject
- * and body (for emails), or an error message
- * @deprecated
- */
- public static function formatNotification( EchoEvent $event, User
$user, $format = 'text', $type = 'web' ) {
- wfDeprecated( 'EchoNotificationController::formatNotification',
'1.28', 'Echo' );
- $eventType = $event->getType();
-
- $res = '';
- try {
- $formatter = EchoNotificationFormatter::factory(
$eventType );
- $formatter->setOutputFormat( $format );
- } catch ( InvalidArgumentException $e ) {
- self::failFormatting( $event, $user );
-
- return '';
- }
- set_error_handler( array( __CLASS__, 'formatterErrorHandler' ),
-1 );
- try {
- $res = $formatter->format( $event, $user, $type );
- } catch ( Exception $e ) {
- $context = array(
- 'id' => $event->getId(),
- 'eventType' => $eventType,
- 'format' => $format,
- 'type' => $type,
- 'user' => $user ? $user->getName() : 'no user',
- 'exceptionName' => get_class( $e ),
- 'exceptionMessage' => $e->getMessage(),
- );
- LoggerFactory::getInstance( 'Echo' )->error( 'Error
formatting notification', $context );
- MWExceptionHandler::logException( $e );
- }
- restore_error_handler();
-
- if ( $res === '' ) {
- self::failFormatting( $event, $user );
- }
-
- return $res;
}
/**
diff --git a/includes/formatters/EchoEventFormatter.php
b/includes/formatters/EchoEventFormatter.php
index 78b34fe..46dc18c 100644
--- a/includes/formatters/EchoEventFormatter.php
+++ b/includes/formatters/EchoEventFormatter.php
@@ -1,4 +1,5 @@
<?php
+use MediaWiki\Logger\LoggerFactory;
/**
* Abstract class that each "formatter" should implement.
@@ -44,6 +45,16 @@
return false;
}
+ if ( !EchoEventPresentationModel::supportsPresentationModel(
$event->getType() ) ) {
+ LoggerFactory::getInstance( 'Echo' )->warning(
+ "Ignoring event type \"{type}\" since it does
not support Echo presentation model.",
+ array(
+ 'type' => $event->getType(),
+ )
+ );
+ return false;
+ }
+
$model = EchoEventPresentationModel::factory( $event,
$this->language, $this->user );
if ( !$model->canRender() ) {
return false;
diff --git a/includes/formatters/EchoFlyoutFormatter.php
b/includes/formatters/EchoFlyoutFormatter.php
index 63fe16b..81dcac5 100644
--- a/includes/formatters/EchoFlyoutFormatter.php
+++ b/includes/formatters/EchoFlyoutFormatter.php
@@ -69,7 +69,7 @@
}
private function getIconURL( EchoEventPresentationModel $model ) {
- return EchoNotificationFormatter::getIconUrl(
+ return EchoIcon::getUrl(
$model->getIconType(),
$this->language->getDir()
);
diff --git a/includes/formatters/EchoHtmlDigestEmailFormatter.php
b/includes/formatters/EchoHtmlDigestEmailFormatter.php
index 77895d0..8db2e11 100644
--- a/includes/formatters/EchoHtmlDigestEmailFormatter.php
+++ b/includes/formatters/EchoHtmlDigestEmailFormatter.php
@@ -175,7 +175,7 @@
*/
protected function applyStyleToEvent( EchoEventPresentationModel $model
) {
$iconUrl = wfExpandUrl(
- EchoNotificationFormatter::getIconUrl(
$model->getIconType(), $this->language->getDir() ),
+ EchoIcon::getUrl( $model->getIconType(),
$this->language->getDir() ),
PROTO_CANONICAL
);
$imgSrc = Sanitizer::encodeAttribute( $iconUrl );
diff --git a/includes/formatters/EchoHtmlEmailFormatter.php
b/includes/formatters/EchoHtmlEmailFormatter.php
index 2792a38..c99eb51 100644
--- a/includes/formatters/EchoHtmlEmailFormatter.php
+++ b/includes/formatters/EchoHtmlEmailFormatter.php
@@ -26,7 +26,7 @@
}
$iconUrl = wfExpandUrl(
- EchoNotificationFormatter::getIconUrl(
$model->getIconType(), $this->language->getDir() ),
+ EchoIcon::getUrl( $model->getIconType(),
$this->language->getDir() ),
PROTO_CANONICAL
);
diff --git a/includes/formatters/EchoIcon.php b/includes/formatters/EchoIcon.php
new file mode 100644
index 0000000..63d0e86
--- /dev/null
+++ b/includes/formatters/EchoIcon.php
@@ -0,0 +1,49 @@
+<?php
+
+class EchoIcon {
+
+ /**
+ * @param string $icon Name of icon as registered in
BeforeCreateEchoEvent hook
+ * @param string $dir either 'ltr' or 'rtl'
+ * @return string
+ */
+ public static function getUrl( $icon, $dir ) {
+ global $wgEchoNotificationIcons, $wgExtensionAssetsPath;
+ if ( !isset( $wgEchoNotificationIcons[$icon] ) ) {
+ throw new InvalidArgumentException( "The $icon icon is
not registered" );
+ }
+
+ $iconInfo = $wgEchoNotificationIcons[$icon];
+ $needsPrefixing = true;
+
+ // Now we need to check it has a valid url/path
+ if ( isset( $iconInfo['url'] ) && $iconInfo['url'] ) {
+ $iconUrl = $iconInfo['url'];
+ $needsPrefixing = false;
+ } elseif ( isset( $iconInfo['path'] ) && $iconInfo['path'] ) {
+ $iconUrl = $iconInfo['path'];
+ } else {
+ // Fallback to hardcoded 'placeholder'. This is used if
someone
+ // doesn't configure the 'site' icon for example.
+ $icon = 'placeholder';
+ $iconUrl =
$wgEchoNotificationIcons['placeholder']['path'];
+ }
+
+ // Might be an array with different icons for ltr/rtl
+ if ( is_array( $iconUrl ) ) {
+ if ( !isset( $iconUrl[$dir] ) ) {
+ throw new UnexpectedValueException( "Icon type
$icon doesn't have an icon for $dir directionality" );
+ }
+
+ $iconUrl = $iconUrl[$dir];
+ }
+
+ // And if it was a 'path', stick the assets path in front
+ if ( $needsPrefixing ) {
+ $iconUrl = "$wgExtensionAssetsPath/$iconUrl";
+ }
+
+ return $iconUrl;
+ }
+
+}
\ No newline at end of file
diff --git a/includes/formatters/EchoModelFormatter.php
b/includes/formatters/EchoModelFormatter.php
index be617ec..4d4050b 100644
--- a/includes/formatters/EchoModelFormatter.php
+++ b/includes/formatters/EchoModelFormatter.php
@@ -11,7 +11,7 @@
*/
protected function formatModel( EchoEventPresentationModel $model ) {
$data = $model->jsonSerialize();
- $data['iconUrl'] = EchoNotificationFormatter::getIconUrl(
$model->getIconType(), $this->language->getDir() );
+ $data['iconUrl'] = EchoIcon::getUrl( $model->getIconType(),
$this->language->getDir() );
if ( isset( $data['links']['primary']['url'] ) ) {
$data['links']['primary']['url'] = wfExpandUrl(
$data['links']['primary']['url'] );
diff --git a/includes/formatters/NotificationFormatter.php
b/includes/formatters/NotificationFormatter.php
deleted file mode 100644
index f90804a..0000000
--- a/includes/formatters/NotificationFormatter.php
+++ /dev/null
@@ -1,193 +0,0 @@
-<?php
-
-/**
- * Abstract class for constructing a notification message, this class includes
- * only the most generic formatting functionality as it may be extended by
- * notification formatters for other extensions with unique content or
- * requirements.
- * @deprecated
- */
-abstract class EchoNotificationFormatter {
-
- /**
- * List of valid output format
- * @var array
- */
- protected $validOutputFormats = array( 'text', 'email', 'htmlemail' );
-
- /**
- * List of valid distribution type
- */
- protected $validDistributionType = array( 'web', 'email',
'emaildigest', 'emailsubject' );
-
- /**
- * Current output format, default is 'text'
- * @var string
- */
- protected $outputFormat = 'text';
-
- /**
- * Distribution type, default is 'web'
- * @var string
- */
- protected $distributionType = 'web';
-
- /**
- * List of parameters for constructing messages
- * @var array
- */
- protected $parameters;
-
- /**
- * Creates an instance of the given class with the given parameters.
- * @param $parameters array Associative array of parameters
- * @throws MWException
- */
- public function __construct( array $parameters ) {
- wfDeprecated( __CLASS__, '1.28', 'Echo' );
- $this->parameters = $parameters;
- }
-
- /**
- * Shows a notification in human-readable format.
- * @param $event EchoEvent being notified about.
- * @param $user User being notified.
- * @param $type string The notification type (e.g. notify, email)
- * @return Mixed; depends on output format
- * @see EchoNotificationFormatter::setOutputFormat
- */
- abstract public function format( $event, $user, $type );
-
- /**
- * Set the output format that the notification will be displayed in.
- * @param $format string A valid output format (by default, 'text', and
'email' are allowed)
- * @throws InvalidArgumentException
- */
- public function setOutputFormat( $format ) {
- if ( !in_array( $format, $this->validOutputFormats, true ) ) {
- throw new InvalidArgumentException( "Invalid output
format $format" );
- }
-
- $this->outputFormat = $format;
- }
-
- public function setDistributionType( $type ) {
- if ( !in_array( $type, $this->validDistributionType, true ) ) {
- throw new InvalidArgumentException( "Invalid
distribution type $type" );
- }
-
- $this->distributionType = $type;
- }
-
- /**
- * Create an EchoNotificationFormatter for the given type.
- * @param string $type
- * Select the class of formatter to use with the 'formatter-class'
field.
- * For other parameters, see the appropriate class' constructor.
- * @throws RuntimeException
- * @return EchoNotificationFormatter object.
- */
- public static function factory( $type ) {
- global $wgEchoNotifications;
- if ( !isset( $wgEchoNotifications[$type] ) ) {
- throw new InvalidArgumentException( "The notification
type '$type' is not registered" );
- }
-
- $parameters = $wgEchoNotifications[$type];
- if ( isset( $parameters['formatter-class'] ) ) {
- $class = $parameters['formatter-class'];
- } else {
- $class = 'EchoBasicFormatter';
- }
-
- if ( !class_exists( $class ) ) {
- throw new RuntimeException( "Class $class does not
exist" );
- }
-
- return new $class( $parameters );
- }
-
- /**
- * Returns a link to a title, or the title itself.
- * @param $title Title object
- * @return string Text suitable for output format
- */
- protected function formatTitle( Title $title ) {
- return $title->getPrefixedText();
- }
-
- /**
- * Formats a timestamp in a human-readable format
- * @param $ts string Timestamp in some format compatible with
wfTimestamp()
- * @return string Human-readable timestamp
- */
- protected function formatTimestamp( $ts ) {
- $timestamp = new MWTimestamp( $ts );
- $ts = $timestamp->getHumanTimestamp();
-
- return $ts;
- }
-
- /**
- * @todo this shouldn't be static
- * @param string $icon Name of icon as registered in
BeforeCreateEchoEvent hook
- * @param string $dir either 'ltr' or 'rtl'
- * @return string
- */
- public static function getIconUrl( $icon, $dir ) {
- global $wgEchoNotificationIcons, $wgExtensionAssetsPath;
- if ( !isset( $wgEchoNotificationIcons[$icon] ) ) {
- throw new InvalidArgumentException( "The $icon icon is
not registered" );
- }
-
- $iconInfo = $wgEchoNotificationIcons[$icon];
- $needsPrefixing = true;
-
- // Now we need to check it has a valid url/path
- if ( isset( $iconInfo['url'] ) && $iconInfo['url'] ) {
- $iconUrl = $iconInfo['url'];
- $needsPrefixing = false;
- } elseif ( isset( $iconInfo['path'] ) && $iconInfo['path'] ) {
- $iconUrl = $iconInfo['path'];
- } else {
- // Fallback to hardcoded 'placeholder'. This is used if
someone
- // doesn't configure the 'site' icon for example.
- $icon = 'placeholder';
- $iconUrl =
$wgEchoNotificationIcons['placeholder']['path'];
- }
-
- // Might be an array with different icons for ltr/rtl
- if ( is_array( $iconUrl ) ) {
- if ( !isset( $iconUrl[$dir] ) ) {
- throw new UnexpectedValueException( "Icon type
$icon doesn't have an icon for $dir directionality" );
- }
-
- $iconUrl = $iconUrl[$dir];
- }
-
- // And if it was a 'path', stick the assets path in front
- if ( $needsPrefixing ) {
- $iconUrl = "$wgExtensionAssetsPath/$iconUrl";
- }
-
- return $iconUrl;
- }
-
- /**
- * Returns a revision snippet
- * @param EchoEvent $event The event that the notification is for.
- * @param User $user The user to format the notification for.
- * @return String The revision snippet (or empty string)
- */
- public function getRevisionSnippet( $event, $user ) {
- $extra = $event->getExtra();
- if ( !isset( $extra['section-text'] ) || !$event->userCan(
Revision::DELETED_TEXT, $user ) ) {
- return '';
- }
-
- $snippet = trim( $extra['section-text'] );
-
- return $snippet;
- }
-
-}
diff --git a/includes/formatters/SpecialNotificationsFormatter.php
b/includes/formatters/SpecialNotificationsFormatter.php
index 7d4fe2c..394fe2f 100644
--- a/includes/formatters/SpecialNotificationsFormatter.php
+++ b/includes/formatters/SpecialNotificationsFormatter.php
@@ -105,7 +105,7 @@
}
private function getIconURL( EchoEventPresentationModel $model ) {
- return EchoNotificationFormatter::getIconUrl(
+ return EchoIcon::getUrl(
$model->getIconType(),
$this->language->getDir()
);
diff --git a/tests/phpunit/formatters/NotificationFormatterTest.php
b/tests/phpunit/formatters/NotificationFormatterTest.php
deleted file mode 100644
index afa7971..0000000
--- a/tests/phpunit/formatters/NotificationFormatterTest.php
+++ /dev/null
@@ -1,115 +0,0 @@
-<?php
-
-/**
- * @group Echo
- */
-class EchoNotificationFormatterTest extends MediaWikiTestCase {
-
- protected function setUp() {
- parent::setUp();
- $user = new User();
- $user->setName( 'Notification-formatter-test' );
- $user->addToDatabase();
- $this->setMwGlobals( 'wgUser', $user );
- }
-
- protected function format( EchoEvent $event, $format, $user = false,
$type = 'web' ) {
- if ( $user === false ) {
- $user = User::newFromName(
'Notification-formatter-test' );
- }
-
- // Notification users can not be anonymous, use a fake user id
- return EchoNotificationController::formatNotification( $event,
$user, $format, $type );
- }
-
- protected function mockEvent( $type, array $extra = array(), Revision
$rev = null, User $agent = null ) {
- $methods = get_class_methods( 'EchoEvent' );
- $methods = array_diff( $methods, array( 'userCan',
'getLinkMessage', 'getLinkDestination' ) );
-
- $event = $this->getMockBuilder( 'EchoEvent' )
- ->disableOriginalConstructor()
- ->setMethods( $methods )
- ->getMock();
- $event->expects( $this->any() )
- ->method( 'getType' )
- ->will( $this->returnValue( $type ) );
- $event->expects( $this->any() )
- ->method( 'getExtra' )
- ->will( $this->returnValue( $extra ) );
- if ( $agent !== null ) {
- $event->expects( $this->any() )
- ->method( 'getAgent' )
- ->will( $this->returnValue( $agent ) );
- }
- if ( $rev !== null ) {
- $event->expects( $this->any() )
- ->method( 'getRevision' )
- ->will( $this->returnValue( $rev ) );
- }
-
- return $event;
- }
-
- /**
- * @dataProvider provideGetIconUrl
- */
- public function testGetIconUrl( $global, $icon, $dir, $expected,
$exception = null ) {
- $this->setMwGlobals( array(
- 'wgEchoNotificationIcons' => $global,
- 'wgExtensionAssetsPath' => 'http://example.org'
- ) );
-
- if ( $exception ) {
- $this->setExpectedException( $exception );
- }
-
- $url = EchoNotificationFormatter::getIconUrl( $icon, $dir );
- $this->assertEquals( $expected, $url );
- }
-
- public static function provideGetIconUrl() {
- $standard = array(
- 'foo' => array(
- 'path' => 'foo.png',
- ),
- 'bar' => array(
- 'path' => array(
- 'ltr' => 'bar.png'
- ),
- ),
- 'baz' => array(
- 'path' => array(
- 'ltr' => 'baz-ltr.png',
- 'rtl' => 'baz-rtl.png',
- ),
- ),
- 'site' => array(
- 'url' => false,
- ),
- 'placeholder' => array(
- 'path' => 'placeholder.png',
- ),
- );
- $tests = array(
- // Standard, no ltr/rtl
- array( $standard, 'foo', 'ltr',
'http://example.org/foo.png' ),
- array( $standard, 'foo', 'rtl',
'http://example.org/foo.png' ),
- // Only ltr configured (bad!)
- array( $standard, 'bar', 'ltr',
'http://example.org/bar.png' ),
- array( $standard, 'bar', 'rtl', '',
'UnexpectedValueException' ),
- // Different for ltr/rtl
- array( $standard, 'baz', 'ltr',
'http://example.org/baz-ltr.png' ),
- array( $standard, 'baz', 'rtl',
'http://example.org/baz-rtl.png' ),
- // Not registered
- array( $standard, 'invalid', 'ltr', '',
'InvalidArgumentException' ),
- // No url set, fallback to placeholder
- array( $standard, 'site', 'ltr',
'http://example.org/placeholder.png' ),
- );
-
- // Set the 'url', and it doesn't fallback anymore
- $standard['site']['url'] = 'http://example.com/site.png';
- $tests[] = array( $standard, 'site', 'ltr',
'http://example.com/site.png' );
-
- return $tests;
- }
-}
--
To view, visit https://gerrit.wikimedia.org/r/300032
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic5712c4ce265b6faabce7a4028b4294fe3c73f18
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: Sbisson <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
