Inject services into Hooks

Change-Id: I96b8c9a83a2ec1b3b517106530f9e68e35cc0c28

extension.json

@@ -21,7 +21,12 @@
 	},
 	"HookHandlers": {
 		"main": {
-			"class": "CookieWarning\\Hooks"
+			"class": "CookieWarning\\Hooks",
+			"services": [
+				"CookieWarning.Config",
+				"CookieWarning.Decisions",
+				"UserOptionsManager"
+			]
 		}
 	},
 	"Hooks": {

includes/Hooks.php

@@ -9,9 +9,9 @@ use MediaWiki;
 use MediaWiki\Hook\BeforeInitializeHook;
 use MediaWiki\Hook\BeforePageDisplayHook;
 use MediaWiki\Hook\SkinAfterContentHook;
-use MediaWiki\MediaWikiServices;
 use MediaWiki\Preferences\Hook\GetPreferencesHook;
 use MediaWiki\ResourceLoader\Hook\ResourceLoaderGetConfigVarsHook;
+use MediaWiki\User\Options\UserOptionsManager;
 use MWException;
 use OOUI\ButtonInputWidget;
 use OOUI\ButtonWidget;
@@ -29,6 +29,20 @@ class Hooks implements
 	BeforePageDisplayHook,
 	ResourceLoaderGetConfigVarsHook
 {
+	private Config $config;
+	private Decisions $decisions;
+	private UserOptionsManager $userOptionsManager;
+
+	public function __construct(
+		Config $config,
+		Decisions $decisions,
+		UserOptionsManager $userOptionsManager
+	) {
+		$this->config = $config;
+		$this->decisions = $decisions;
+		$this->userOptionsManager = $userOptionsManager;
+	}
+
 	/**
 	 * BeforeInitialize hook handler.
 	 *
@@ -49,9 +63,8 @@ class Hooks implements
 		}
 
 		if ( $user->isRegistered() ) {
-			$userOptionsManager = MediaWikiServices::getInstance()->getUserOptionsManager();
+			$this->userOptionsManager->setOption( $user, 'cookiewarning_dismissed', 1 );
-			$userOptionsManager->setOption( $user, 'cookiewarning_dismissed', 1 );
+			$this->userOptionsManager->saveOptions( $user );
-			$userOptionsManager->saveOptions( $user );
 		} else {
 			$request->response()->setCookie( 'cookiewarning_dismissed', true );
 		}
@@ -69,15 +82,11 @@ class Hooks implements
 	 * @throws MWException
 	 */
 	public function onSkinAfterContent( &$data, $skin ) {
-		/** @var Decisions $cookieWarningDecisions */
+		if ( !$this->decisions->shouldShowCookieWarning( $skin->getContext() ) ) {
-		$cookieWarningDecisions = MediaWikiServices::getInstance()
-			->getService( 'CookieWarning.Decisions' );
-
-		if ( !$cookieWarningDecisions->shouldShowCookieWarning( $skin->getContext() ) ) {
 			return;
 		}
 
-		$data .= self::generateElements( $skin );
+		$data .= $this->generateElements( $skin );
 	}
 
 	/**
@@ -86,8 +95,8 @@ class Hooks implements
 	 * @param Skin $skin
 	 * @return string|null The html for cookie notice.
 	 */
-	private static function generateElements( Skin $skin ): ?string {
+	private function generateElements( Skin $skin ): ?string {
-		$moreLink = self::getMoreLink();
+		$moreLink = $this->getMoreLink();
 
 		$buttons = [];
 		if ( $moreLink ) {
@@ -139,10 +148,9 @@ class Hooks implements
 	 * @return string|null The url or null if none set
 	 * @throws ConfigException
 	 */
-	private static function getMoreLink(): ?string {
+	private function getMoreLink(): ?string {
-		$conf = self::getConfig();
+		if ( $this->config->get( 'CookieWarningMoreUrl' ) ) {
-		if ( $conf->get( 'CookieWarningMoreUrl' ) ) {
+			return $this->config->get( 'CookieWarningMoreUrl' );
-			return $conf->get( 'CookieWarningMoreUrl' );
 		}
 
 		$cookieWarningMessage = wfMessage( 'cookiewarning-more-link' );
@@ -169,18 +177,14 @@ class Hooks implements
 	 * @throws MWException
 	 */
 	public function onBeforePageDisplay( $out, $skin ): void {
-		/** @var Decisions $cookieWarningDecisions */
+		if ( !$this->decisions->shouldShowCookieWarning( $out->getContext() ) ) {
-		$cookieWarningDecisions = MediaWikiServices::getInstance()
-			->getService( 'CookieWarning.Decisions' );
-
-		if ( !$cookieWarningDecisions->shouldShowCookieWarning( $out->getContext() ) ) {
 			return;
 		}
 
 		$modules = [ 'ext.CookieWarning' ];
 		$moduleStyles = [ 'ext.CookieWarning.styles' ];
 
-		if ( $cookieWarningDecisions->shouldAddResourceLoaderComponents() ) {
+		if ( $this->decisions->shouldAddResourceLoaderComponents() ) {
 			$modules[] = 'ext.CookieWarning.geolocation';
 			$moduleStyles[] = 'ext.CookieWarning.geolocation.styles';
 		}
@@ -199,28 +203,14 @@ class Hooks implements
 	 * @throws ConfigException
 	 */
 	public function onResourceLoaderGetConfigVars( array &$vars, $skin, Config $config ): void {
-		/** @var Decisions $cookieWarningDecisions */
+		if ( $this->decisions->shouldAddResourceLoaderComponents() ) {
-		$cookieWarningDecisions = MediaWikiServices::getInstance()
-			->getService( 'CookieWarning.Decisions' );
-		$conf = self::getConfig();
-
-		if ( $cookieWarningDecisions->shouldAddResourceLoaderComponents() ) {
 			$vars += [
-				'wgCookieWarningGeoIPServiceURL' => $conf->get( 'CookieWarningGeoIPServiceURL' ),
+				'wgCookieWarningGeoIPServiceURL' => $this->config->get( 'CookieWarningGeoIPServiceURL' ),
-				'wgCookieWarningForCountryCodes' => $conf->get( 'CookieWarningForCountryCodes' ),
+				'wgCookieWarningForCountryCodes' => $this->config->get( 'CookieWarningForCountryCodes' ),
 			];
 		}
 	}
 
-	/**
-	 * Returns the Config object for the CookieWarning extension.
-	 *
-	 * @return Config
-	 */
-	private static function getConfig(): Config {
-		return MediaWikiServices::getInstance()->getService( 'CookieWarning.Config' );
-	}
-
 	/**
 	 * GetPreferences hook handler
 	 *

tests/phpunit/includes/HooksTest.php

@@ -16,6 +16,15 @@ use SkinTemplate;
  */
 class HooksTest extends MediaWikiLangTestCase {
 
+	private function newHooks(): Hooks {
+		$services = $this->getServiceContainer();
+		return new Hooks(
+			$services->getService( 'CookieWarning.Config' ),
+			$services->getService( 'CookieWarning.Decisions' ),
+			$services->getUserOptionsManager()
+		);
+	}
+
 	/**
 	 * @dataProvider providerOnSiteNoticeAfter
 	 *
@@ -65,7 +74,7 @@ class HooksTest extends MediaWikiLangTestCase {
 		$sk->getOutput()->enableOOUI();
 
 		$data = '';
-		( new Hooks() )->onSkinAfterContent( $data, $sk );
+		$this->newHooks()->onSkinAfterContent( $data, $sk );
 
 		if ( $enabled ) {
 			$this->assertNotSame( '', $data, 'Cookie warning should be present' );
@@ -178,7 +187,7 @@ class HooksTest extends MediaWikiLangTestCase {
 		$sk = new SkinTemplate();
 		$sk->setContext( $context );
 		$data = '';
-		( new Hooks() )->onSkinAfterContent( $data, $sk );
+		$this->newHooks()->onSkinAfterContent( $data, $sk );
 
 		$this->assertEquals(
 			$expected,