Aude has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/382386 )
Change subject: Split hook handler that runs on Echo extension setup ...................................................................... Split hook handler that runs on Echo extension setup BeforeCreateEchoEvent hook runs on Echo extension initialization, so on every page load. This hook doesn't need any service objects... only a couple settings, vs other Echo hooks that run at other times and use services. Having the shared newFromGlobalState for all these hooks means initializing unnecessary services on every page load. This patch splits the hook handling into two hook handler classes, depending on services needed. Bug: T177311 Change-Id: Id1fe10bcbf79fd67f36b3d6f0e716048b32af216 --- M client/WikibaseClient.php M client/includes/Hooks/EchoNotificationsHandlers.php A client/includes/Hooks/EchoSetupHookHandlers.php M client/tests/phpunit/includes/Hooks/EchoNotificationsHandlersTest.php A client/tests/phpunit/includes/Hooks/EchoSetupHookHandlersTest.php M client/tests/phpunit/includes/Notifications/PageConnectionPresentationModelTest.php 6 files changed, 202 insertions(+), 157 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase refs/changes/86/382386/1 diff --git a/client/WikibaseClient.php b/client/WikibaseClient.php index cf3ac87..352da9f 100644 --- a/client/WikibaseClient.php +++ b/client/WikibaseClient.php @@ -151,7 +151,7 @@ // for client notifications (requires the Echo extension) // note that Echo calls BeforeCreateEchoEvent hook when it is being initialized, // thus we have to register these two handlers disregarding Echo is loaded or not - $wgHooks['BeforeCreateEchoEvent'][] = '\Wikibase\Client\Hooks\EchoNotificationsHandlers::onBeforeCreateEchoEvent'; + $wgHooks['BeforeCreateEchoEvent'][] = '\Wikibase\Client\Hooks\EchoSetupHookHandlers::onBeforeCreateEchoEvent'; $wgHooks['EchoGetBundleRules'][] = '\Wikibase\Client\Hooks\EchoNotificationsHandlers::onEchoGetBundleRules'; // conditionally register the remaining two handlers which would otherwise fail diff --git a/client/includes/Hooks/EchoNotificationsHandlers.php b/client/includes/Hooks/EchoNotificationsHandlers.php index 60bbf33..f4110d2 100644 --- a/client/includes/Hooks/EchoNotificationsHandlers.php +++ b/client/includes/Hooks/EchoNotificationsHandlers.php @@ -52,11 +52,6 @@ private $sendEchoNotification; /** - * @var array|false - */ - private $echoIcon; - - /** * @var string */ private $repoSiteName; @@ -66,7 +61,6 @@ * @param NamespaceChecker $namespaceChecker * @param string $siteId * @param bool $sendEchoNotification - * @param array|false $echoIcon * @param string $repoSiteName */ public function __construct( @@ -74,14 +68,12 @@ NamespaceChecker $namespaceChecker, $siteId, $sendEchoNotification, - $echoIcon, $repoSiteName ) { $this->repoLinker = $repoLinker; $this->namespaceChecker = $namespaceChecker; $this->siteId = $siteId; $this->sendEchoNotification = $sendEchoNotification; - $this->echoIcon = $echoIcon; $this->repoSiteName = $repoSiteName; } @@ -97,68 +89,8 @@ $wikibaseClient->getNamespaceChecker(), $settings->getSetting( 'siteGlobalID' ), $settings->getSetting( 'sendEchoNotification' ), - $settings->getSetting( 'echoIcon' ), $settings->getSetting( 'repoSiteName' ) ); - } - - /** - * Handler for BeforeCreateEchoEvent hook - * @see https://www.mediawiki.org/wiki/Extension:Echo/BeforeCreateEchoEvent - * @see doBeforeCreateEchoEvent - * - * @param array[] &$notifications - * @param array[] &$notificationCategories - * @param array[] &$icons - */ - public static function onBeforeCreateEchoEvent( - array &$notifications, - array &$notificationCategories, - array &$icons - ) { - $self = self::newFromGlobalState(); - $self->doBeforeCreateEchoEvent( $notifications, $notificationCategories, $icons ); - } - - /** - * @see https://www.mediawiki.org/wiki/Notifications/Developer_guide - * - * @param array[] &$notifications - * @param array[] &$notificationCategories - * @param array[] &$icons - */ - public function doBeforeCreateEchoEvent( - array &$notifications, - array &$notificationCategories, - array &$icons - ) { - if ( $this->sendEchoNotification !== true ) { - return; - } - - $notificationCategories['wikibase-action'] = [ - 'priority' => 5, - 'tooltip' => 'echo-pref-tooltip-wikibase-action', - ]; - - $notifications[self::NOTIFICATION_TYPE] = [ - EchoAttributeManager::ATTR_LOCATORS => [ - EchoUserLocator::class . '::locateArticleCreator', - ], - 'category' => 'wikibase-action', - 'group' => 'neutral', - 'section' => 'message', - 'presentation-model' => PageConnectionPresentationModel::class, - 'bundle' => [ 'web' => true, 'email' => false ], - ]; - - if ( !empty( $this->echoIcon ) ) { - $icons[self::NOTIFICATION_TYPE] = $this->echoIcon; - } else { - preg_match( '+/extensions/(.*)+', __DIR__, $remoteExtPath ); - $iconPath = $remoteExtPath[1] . '/../../resources/images/echoIcon.svg'; - $icons[self::NOTIFICATION_TYPE] = [ 'path' => $iconPath ]; - } } /** diff --git a/client/includes/Hooks/EchoSetupHookHandlers.php b/client/includes/Hooks/EchoSetupHookHandlers.php new file mode 100644 index 0000000..7adacac --- /dev/null +++ b/client/includes/Hooks/EchoSetupHookHandlers.php @@ -0,0 +1,122 @@ +<?php + +namespace Wikibase\Client\Hooks; + +use Diff\DiffOp\DiffOp; +use Diff\DiffOp\DiffOpAdd; +use Diff\DiffOp\DiffOpChange; +use EchoAttributeManager; +use EchoEvent; +use EchoUserLocator; +use Title; +use User; +use Wikibase\Change; +use Wikibase\Client\NamespaceChecker; +use Wikibase\Client\Notifications\PageConnectionPresentationModel; +use Wikibase\Client\RepoLinker; +use Wikibase\Client\WikibaseClient; +use Wikibase\ItemChange; +use WikiPage; + +/** + * Handlers for hooks (e.g. BeforeCreateEchoEvent) called when Echo extension + * is initialized, so on every page load. + * + * @license GPL-2.0+ + * @author Matěj Suchánek + * @author Katie Filbert < aude.w...@gmail.com > + */ +class EchoSetupHookHandlers { + + /** + * @var bool + */ + private $sendEchoNotification; + + /** + * @var array|false + */ + private $echoIcon; + + /** + * @param bool $sendEchoNotification + * @param array|false $echoIcon + */ + public function __construct( $sendEchoNotification, $echoIcon ) { + $this->sendEchoNotification = $sendEchoNotification; + $this->echoIcon = $echoIcon; + } + + /** + * @return self + */ + public static function newFromGlobalState() { + $wikibaseClient = WikibaseClient::getDefaultInstance(); + $settings = $wikibaseClient->getSettings(); + + return new self( + $settings->getSetting( 'sendEchoNotification' ), + $settings->getSetting( 'echoIcon' ) + ); + } + + /** + * Handler for BeforeCreateEchoEvent hook + * @see https://www.mediawiki.org/wiki/Extension:Echo/BeforeCreateEchoEvent + * @see doBeforeCreateEchoEvent + * + * @param array[] &$notifications + * @param array[] &$notificationCategories + * @param array[] &$icons + */ + public static function onBeforeCreateEchoEvent( + array &$notifications, + array &$notificationCategories, + array &$icons + ) { + $self = self::newFromGlobalState(); + $self->doBeforeCreateEchoEvent( $notifications, $notificationCategories, $icons ); + } + + /** + * @see https://www.mediawiki.org/wiki/Notifications/Developer_guide + * + * @param array[] &$notifications + * @param array[] &$notificationCategories + * @param array[] &$icons + */ + public function doBeforeCreateEchoEvent( + array &$notifications, + array &$notificationCategories, + array &$icons + ) { + if ( $this->sendEchoNotification !== true ) { + return; + } + + $notificationCategories['wikibase-action'] = [ + 'priority' => 5, + 'tooltip' => 'echo-pref-tooltip-wikibase-action', + ]; + + $notifications[EchoNotificationsHandlers::NOTIFICATION_TYPE] = [ + EchoAttributeManager::ATTR_LOCATORS => [ + EchoUserLocator::class . '::locateArticleCreator', + ], + 'category' => 'wikibase-action', + 'group' => 'neutral', + 'section' => 'message', + 'presentation-model' => PageConnectionPresentationModel::class, + 'bundle' => [ 'web' => true, 'email' => false ], + ]; + + if ( !empty( $this->echoIcon ) ) { + $icons[EchoNotificationsHandlers::NOTIFICATION_TYPE] = $this->echoIcon; + } else { + preg_match( '+/extensions/(.*)+', __DIR__, $remoteExtPath ); + $iconPath = $remoteExtPath[1] . '/../../resources/images/echoIcon.svg'; + $icons[EchoNotificationsHandlers::NOTIFICATION_TYPE] = [ 'path' => $iconPath ]; + } + } + +} diff --git a/client/tests/phpunit/includes/Hooks/EchoNotificationsHandlersTest.php b/client/tests/phpunit/includes/Hooks/EchoNotificationsHandlersTest.php index ef7cd45..68ae3eb 100644 --- a/client/tests/phpunit/includes/Hooks/EchoNotificationsHandlersTest.php +++ b/client/tests/phpunit/includes/Hooks/EchoNotificationsHandlersTest.php @@ -67,7 +67,6 @@ $this->namespaceChecker, $settings->getSetting( 'siteGlobalID' ), $settings->getSetting( 'sendEchoNotification' ), - $settings->getSetting( 'echoIcon' ), 'repoSiteName' ); } @@ -174,68 +173,6 @@ ); } - public function beforeCreateEchoEventProvider() { - return [ - 'no registration' => [ - 'register' => false, - 'icon' => false, - 'expectedIcon' => false, - ], - 'registered with optional icon' => [ - 'register' => true, - 'icon' => [ 'url' => 'some_url_here' ], - 'expectedIcon' => [ 'url' => 'some_url_here' ], - ], - 'registered with default icon' => [ - 'register' => true, - 'icon' => false, - 'expectedIcon' => [ 'path' => 'Wikibase/client/includes/Hooks/../../resources/images/echoIcon.svg' ], - ] - ]; - } - - /** - * @dataProvider beforeCreateEchoEventProvider - */ - public function testBeforeCreateEchoEvent( $register, $icon, $expectedIcon ) { - $notifications = []; - $categories = []; - $icons = []; - - $handlers = new EchoNotificationsHandlers( - $this->repoLinker, - $this->namespaceChecker, - 'enwiki', - $register, - $icon, - 'repoSiteName' - ); - - $handlers->doBeforeCreateEchoEvent( $notifications, $categories, $icons ); - - $this->assertSame( $register, isset( $notifications[$handlers::NOTIFICATION_TYPE] ) ); - $this->assertSame( $register, isset( $categories['wikibase-action'] ) ); - $this->assertSame( $register, isset( $icons[$handlers::NOTIFICATION_TYPE] ) ); - - if ( $register ) { - if ( isset( $expectedIcon['path'] ) ) { - $this->assertSame( - array_keys( $expectedIcon ), - array_keys( $icons[$handlers::NOTIFICATION_TYPE] ) - ); - $this->assertStringEndsWith( - $expectedIcon['path'], - $icons[$handlers::NOTIFICATION_TYPE]['path'] - ); - } else { - $this->assertSame( - $expectedIcon, - $icons[$handlers::NOTIFICATION_TYPE] - ); - } - } - } - public function localUserCreatedProvider() { return [ 'disabled no auto' => [ @@ -270,7 +207,6 @@ $this->namespaceChecker, 'enwiki', $enabled, - '', 'repoSiteName' ); diff --git a/client/tests/phpunit/includes/Hooks/EchoSetupHookHandlersTest.php b/client/tests/phpunit/includes/Hooks/EchoSetupHookHandlersTest.php new file mode 100644 index 0000000..7cdf634 --- /dev/null +++ b/client/tests/phpunit/includes/Hooks/EchoSetupHookHandlersTest.php @@ -0,0 +1,76 @@ +<?php + +namespace Wikibase\Client\Tests\Hooks; + +use EchoEvent; +use MediaWikiTestCase; +use Wikibase\Client\Hooks\EchoSetupHookHandlers; + +/** + * @covers Wikibase\Client\Hooks\EchoSetupHookHandlers + * + * @group Database + * @group WikibaseClient + * @group Wikibase + */ +class EchoSetupHookHandlersTest extends MediaWikiTestCase { + + public function beforeCreateEchoEventProvider() { + return [ + 'no registration' => [ + 'register' => false, + 'icon' => false, + 'expectedIcon' => false, + ], + 'registered with optional icon' => [ + 'register' => true, + 'icon' => [ 'url' => 'some_url_here' ], + 'expectedIcon' => [ 'url' => 'some_url_here' ], + ], + 'registered with default icon' => [ + 'register' => true, + 'icon' => false, + 'expectedIcon' => [ 'path' => 'Wikibase/client/includes/Hooks/../../resources/images/echoIcon.svg' ], + ] + ]; + } + + /** + * @dataProvider beforeCreateEchoEventProvider + */ + public function testBeforeCreateEchoEvent( $register, $icon, $expectedIcon ) { + if ( !class_exists( EchoEvent::class ) ) { + $this->markTestSkipped( "Echo not loaded" ); + } + + $notifications = []; + $categories = []; + $icons = []; + + $handlers = new EchoSetupHookHandlers( $register, $icon ); + + $handlers->doBeforeCreateEchoEvent( $notifications, $categories, $icons ); + + $this->assertSame( $register, isset( $notifications[$handlers::NOTIFICATION_TYPE] ) ); + $this->assertSame( $register, isset( $categories['wikibase-action'] ) ); + $this->assertSame( $register, isset( $icons[$handlers::NOTIFICATION_TYPE] ) ); + + if ( $register ) { + if ( isset( $expectedIcon['path'] ) ) { + $this->assertSame( + array_keys( $expectedIcon ), + array_keys( $icons[$handlers::NOTIFICATION_TYPE] ) + ); + $this->assertStringEndsWith( + $expectedIcon['path'], + $icons[$handlers::NOTIFICATION_TYPE]['path'] + ); + } else { + $this->assertSame( + $expectedIcon, + $icons[$handlers::NOTIFICATION_TYPE] + ); + } + } + } +} diff --git a/client/tests/phpunit/includes/Notifications/PageConnectionPresentationModelTest.php b/client/tests/phpunit/includes/Notifications/PageConnectionPresentationModelTest.php index c2ae5ec..88a28eb 100644 --- a/client/tests/phpunit/includes/Notifications/PageConnectionPresentationModelTest.php +++ b/client/tests/phpunit/includes/Notifications/PageConnectionPresentationModelTest.php @@ -9,6 +9,7 @@ use Title; use User; use Wikibase\Client\Hooks\EchoNotificationsHandlers; +use Wikibase\Client\Hooks\EchoSetupHookHandlers; use Wikibase\Client\NamespaceChecker; use Wikibase\Client\Notifications\PageConnectionPresentationModel; use Wikibase\Client\RepoLinker; @@ -61,34 +62,12 @@ return $event; } - /** - * @return RepoLinker - */ - private function getRepoLinker() { - $repoLinker = $this->getMockBuilder( RepoLinker::class ) - ->disableOriginalConstructor() - ->getMock(); - $repoLinker - ->expects( $this->any() ) - ->method( 'getEntityUrl' ) - ->will( $this->returnValue( 'foo' ) ); - - return $repoLinker; - } - public function testPresentationModel() { global $wgEchoNotifications, $wgEchoNotificationCategories, $wgEchoNotificationIcons; - $namespaceChecker = $this->getMockBuilder( NamespaceChecker::class ) - ->disableOriginalConstructor() - ->getMock(); - $handlers = new EchoNotificationsHandlers( - $this->getRepoLinker(), - $namespaceChecker, - 'enwiki', + $handlers = new EchoSetupHookHandlers( true, - false, - 'repoSiteName' + false ); $handlers->doBeforeCreateEchoEvent( $wgEchoNotifications, $wgEchoNotificationCategories, $wgEchoNotificationIcons -- To view, visit https://gerrit.wikimedia.org/r/382386 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id1fe10bcbf79fd67f36b3d6f0e716048b32af216 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Aude <aude.w...@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits