From 1ce8b38f9fd9cfbc91996b1ed32bb48902fb31e3 Mon Sep 17 00:00:00 2001
From: Fomafix <fomafix@googlemail.com>
Date: Tue, 19 Mar 2024 20:26:53 +0000
Subject: [PATCH] Inject 'Popups.Logger' into PopupsHooks

The function PopupsContext::getLogger is not needed anymore.

Change-Id: Ib538372f1480d282857d827fedbe0ebe9abc0685
---
 extension.json                    |  1 +
 includes/PopupsContext.php        | 10 ----------
 includes/PopupsHooks.php          | 10 ++++++++--
 tests/phpunit/PopupsHooksTest.php | 13 ++++++++++---
 4 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/extension.json b/extension.json
index 51f3b714c..9195cdc30 100644
--- a/extension.json
+++ b/extension.json
@@ -32,6 +32,7 @@
 			"services": [
 				"Popups.Config",
 				"Popups.Context",
+				"Popups.Logger",
 				"UserOptionsManager"
 			]
 		}
diff --git a/includes/PopupsContext.php b/includes/PopupsContext.php
index b97247f33..aea52e138 100644
--- a/includes/PopupsContext.php
+++ b/includes/PopupsContext.php
@@ -22,7 +22,6 @@ namespace Popups;
 
 use ExtensionRegistry;
 use MediaWiki\Config\Config;
-use MediaWiki\MediaWikiServices;
 use MediaWiki\SpecialPage\SpecialPageFactory;
 use MediaWiki\Title\Title;
 use MediaWiki\User\Options\UserOptionsLookup;
@@ -242,13 +241,4 @@ class PopupsContext {
 		}
 		return false;
 	}
-
-	/**
-	 * Get module logger
-	 *
-	 * @return \Psr\Log\LoggerInterface
-	 */
-	public function getLogger() {
-		return MediaWikiServices::getInstance()->getService( 'Popups.Logger' );
-	}
 }
diff --git a/includes/PopupsHooks.php b/includes/PopupsHooks.php
index 10f2260bd..5354e8dc3 100644
--- a/includes/PopupsHooks.php
+++ b/includes/PopupsHooks.php
@@ -32,6 +32,7 @@ use MediaWiki\ResourceLoader\Hook\ResourceLoaderGetConfigVarsHook;
 use MediaWiki\User\Hook\UserGetDefaultOptionsHook;
 use MediaWiki\User\Options\UserOptionsManager;
 use MediaWiki\User\User;
+use Psr\Log\LoggerInterface;
 use Skin;
 
 /**
@@ -56,21 +57,27 @@ class PopupsHooks implements
 	/** @var PopupsContext */
 	private $popupsContext;
 
+	/** @var LoggerInterface */
+	private $logger;
+
 	/** @var UserOptionsManager */
 	private $userOptionsManager;
 
 	/**
 	 * @param Config $config
 	 * @param PopupsContext $popupsContext
+	 * @param LoggerInterface $logger
 	 * @param UserOptionsManager $userOptionsManager
 	 */
 	public function __construct(
 		Config $config,
 		PopupsContext $popupsContext,
+		LoggerInterface $logger,
 		UserOptionsManager $userOptionsManager
 	) {
 		$this->config = $config;
 		$this->popupsContext = $popupsContext;
+		$this->logger = $logger;
 		$this->userOptionsManager = $userOptionsManager;
 	}
 
@@ -196,8 +203,7 @@ class PopupsHooks implements
 		}
 
 		if ( !$this->popupsContext->areDependenciesMet() ) {
-			$logger = $this->popupsContext->getLogger();
-			$logger->error( 'Popups requires the PageImages extensions.
+			$this->logger->error( 'Popups requires the PageImages extensions.
 				TextExtracts extension is required when using mwApiPlain gateway.' );
 			return;
 		}
diff --git a/tests/phpunit/PopupsHooksTest.php b/tests/phpunit/PopupsHooksTest.php
index a8a906ca9..f0a8241f4 100644
--- a/tests/phpunit/PopupsHooksTest.php
+++ b/tests/phpunit/PopupsHooksTest.php
@@ -50,6 +50,7 @@ class PopupsHooksTest extends MediaWikiIntegrationTestCase {
 		( new PopupsHooks(
 			new HashConfig(),
 			$contextMock,
+			$this->getServiceContainer()->getService( 'Popups.Logger' ),
 			$this->getServiceContainer()->getUserOptionsManager()
 		) )
 			->onGetPreferences( $this->createMock( User::class ), $prefs );
@@ -82,6 +83,7 @@ class PopupsHooksTest extends MediaWikiIntegrationTestCase {
 				'PopupsReferencePreviews' => $enabled,
 			] ),
 			$contextMock,
+			$this->getServiceContainer()->getService( 'Popups.Logger' ),
 			$this->getServiceContainer()->getUserOptionsManager()
 		) )
 			->onGetPreferences( $userMock, $prefs );
@@ -122,6 +124,7 @@ class PopupsHooksTest extends MediaWikiIntegrationTestCase {
 				'PopupsReferencePreviews' => $enabled,
 			] ),
 			$contextMock,
+			$this->getServiceContainer()->getService( 'Popups.Logger' ),
 			$this->getServiceContainer()->getUserOptionsManager()
 		) )
 			->onGetPreferences( $this->createMock( User::class ), $prefs );
@@ -161,6 +164,7 @@ class PopupsHooksTest extends MediaWikiIntegrationTestCase {
 				'PopupsReferencePreviews' => $enabled,
 			] ),
 			$contextMock,
+			$this->getServiceContainer()->getService( 'Popups.Logger' ),
 			$this->getServiceContainer()->getUserOptionsManager()
 		) )
 			->onGetPreferences( $this->createMock( User::class ), $prefs );
@@ -189,6 +193,7 @@ class PopupsHooksTest extends MediaWikiIntegrationTestCase {
 		( new PopupsHooks(
 			new HashConfig( $config ),
 			$this->getServiceContainer()->getService( 'Popups.Context' ),
+			$this->getServiceContainer()->getService( 'Popups.Logger' ),
 			$this->getServiceContainer()->getUserOptionsManager()
 		) )
 			->onResourceLoaderGetConfigVars( $vars, '', new MultiConfig( $config ) );
@@ -222,13 +227,11 @@ class PopupsHooksTest extends MediaWikiIntegrationTestCase {
 		$contextMock->expects( $this->once() )
 			->method( 'isTitleExcluded' )
 			->willReturn( false );
-		$contextMock->expects( $this->once() )
-			->method( 'getLogger' )
-			->willReturn( $loggerMock );
 
 		( new PopupsHooks(
 			new HashConfig(),
 			$contextMock,
+			$loggerMock,
 			$this->getServiceContainer()->getUserOptionsManager()
 		) )
 			->onBeforePageDisplay( $outPageMock, $skinMock );
@@ -278,6 +281,7 @@ class PopupsHooksTest extends MediaWikiIntegrationTestCase {
 		( new PopupsHooks(
 			new HashConfig(),
 			$contextMock,
+			$this->getServiceContainer()->getService( 'Popups.Logger' ),
 			$this->getServiceContainer()->getUserOptionsManager()
 		) )
 			->onBeforePageDisplay( $outPageMock, $skinMock );
@@ -302,6 +306,7 @@ class PopupsHooksTest extends MediaWikiIntegrationTestCase {
 		( new PopupsHooks(
 			new HashConfig(),
 			$contextMock,
+			$this->getServiceContainer()->getService( 'Popups.Logger' ),
 			$this->getServiceContainer()->getUserOptionsManager()
 		) )
 			->onMakeGlobalVariablesScript( $vars, $outputPage );
@@ -326,6 +331,7 @@ class PopupsHooksTest extends MediaWikiIntegrationTestCase {
 				'PopupsReferencePreviews' => $enabled,
 			] ),
 			$this->getServiceContainer()->getService( 'Popups.Context' ),
+			$this->getServiceContainer()->getService( 'Popups.Logger' ),
 			$this->getServiceContainer()->getUserOptionsManager()
 		) )
 			->onUserGetDefaultOptions( $userOptions );
@@ -365,6 +371,7 @@ class PopupsHooksTest extends MediaWikiIntegrationTestCase {
 				'PopupsReferencePreviews' => $enabled,
 			] ),
 			$this->getServiceContainer()->getService( 'Popups.Context' ),
+			$this->getServiceContainer()->getService( 'Popups.Logger' ),
 			$userOptionsManagerMock
 		) )
 			->onLocalUserCreated( $userMock, false );