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

Reply via email to