Merge "Inject services into hook handler"

extension.json

@@ -35,10 +35,12 @@
 	},
 	"HookHandlers": {
 		"main": {
-			"class": "PageImages\\PageImages"
+			"class": "PageImages\\PageImages",
+			"services": [ "UserOptionsLookup" ]
 		},
 		"parser": {
-			"class": "PageImages\\Hooks\\ParserFileProcessingHookHandlers"
+			"class": "PageImages\\Hooks\\ParserFileProcessingHookHandlers",
+			"services": [ "RepoGroup", "MainWANObjectCache" ]
 		},
 		"search": {
 			"class": "PageImages\\Hooks\\SearchResultProvideThumbnailHookHandler",

includes/Hooks/ParserFileProcessingHookHandlers.php

@@ -10,13 +10,14 @@ use Http;
 use MediaWiki\Hook\ParserAfterTidyHook;
 use MediaWiki\Hook\ParserModifyImageHTML;
 use MediaWiki\Hook\ParserTestGlobalsHook;
-use MediaWiki\MediaWikiServices;
 use PageImages\PageImageCandidate;
 use PageImages\PageImages;
 use Parser;
 use ParserOutput;
+use RepoGroup;
 use RuntimeException;
 use Title;
+use WANObjectCache;
 
 /**
  * Handlers for parser hooks.
@@ -42,35 +43,22 @@ class ParserFileProcessingHookHandlers implements
 {
 	private const CANDIDATE_REGEX = '/<!--MW-PAGEIMAGES-CANDIDATE-([0-9]+)-->/';
 
-	/**
-	 * ParserModifyImageHTML hook. Save candidate images, and mark them with a
-	 * comment so that we can later tell if they were in the lead section.
-	 *
-	 * @param Parser $parser
-	 * @param File $file
-	 * @param array $params
-	 * @param string &$html
-	 */
-	public function onParserModifyImageHTML(
-		Parser $parser,
-		File $file,
-		array $params,
-		string &$html
-	): void {
-		$handler = new self();
-		$handler->doParserModifyImageHTML( $parser, $file, $params, $html );
-	}
+	/** @var RepoGroup */
+	private $repoGroup;
+
+	/** @var WANObjectCache */
+	private $mainWANObjectCache;
 
 	/**
-	 * ParserAfterTidy hook handler. Remove candidate images which were not in
-	 * the lead section.
-	 *
-	 * @param Parser $parser
-	 * @param string &$text
+	 * @param RepoGroup $repoGroup
+	 * @param WANObjectCache $mainWANObjectCache
 	 */
-	public function onParserAfterTidy( $parser, &$text ) {
-		$handler = new self();
-		$handler->doParserAfterTidy( $parser, $text );
+	public function __construct(
+		RepoGroup $repoGroup,
+		WANObjectCache $mainWANObjectCache
+	) {
+		$this->repoGroup = $repoGroup;
+		$this->mainWANObjectCache = $mainWANObjectCache;
 	}
 
 	/**
@@ -92,17 +80,20 @@ class ParserFileProcessingHookHandlers implements
 	}
 
 	/**
+	 * ParserModifyImageHTML hook. Save candidate images, and mark them with a
+	 * comment so that we can later tell if they were in the lead section.
+	 *
 	 * @param Parser $parser
 	 * @param File $file
 	 * @param array $params
 	 * @param string &$html
 	 */
-	public function doParserModifyImageHTML(
+	public function onParserModifyImageHTML(
 		Parser $parser,
 		File $file,
 		array $params,
 		string &$html
-	) {
+	): void {
 		if ( !$this->processThisTitle( $parser->getTitle() ) ) {
 			return;
 		}
@@ -117,10 +108,13 @@ class ParserFileProcessingHookHandlers implements
 	}
 
 	/**
+	 * ParserAfterTidy hook handler. Remove candidate images which were not in
+	 * the lead section.
+	 *
 	 * @param Parser $parser
 	 * @param string &$text
 	 */
-	public function doParserAfterTidy( Parser $parser, &$text ) {
+	public function onParserAfterTidy( $parser, &$text ) {
 		global $wgPageImagesLeadSectionOnly;
 		$parserOutput = $parser->getOutput();
 		$allImages = $parserOutput->getExtensionData( 'pageImages' );
@@ -354,7 +348,7 @@ class ParserFileProcessingHookHandlers implements
 	 * @return bool
 	 */
 	protected function isImageFree( $fileName ) {
-		$file = MediaWikiServices::getInstance()->getRepoGroup()->findFile( $fileName );
+		$file = $this->repoGroup->findFile( $fileName );
 		if ( $file ) {
 			// Process copyright metadata from CommonsMetadata, if present.
 			// Image is considered free if the value is '0' or unset.
@@ -407,10 +401,8 @@ class ParserFileProcessingHookHandlers implements
 	protected function getDenylist() {
 		global $wgPageImagesDenylistExpiry;
 
-		$cache = MediaWikiServices::getInstance()->getMainWANObjectCache();
-
-		return $cache->getWithSetCallback(
-			$cache->makeKey( 'pageimages-denylist' ),
+		return $this->mainWANObjectCache->getWithSetCallback(
+			$this->mainWANObjectCache->makeKey( 'pageimages-denylist' ),
 			$wgPageImagesDenylistExpiry,
 			function () {
 				global $wgPageImagesDenylist;

includes/PageImages.php

@@ -11,6 +11,7 @@ use MediaWiki\Api\Hook\ApiOpenSearchSuggestHook;
 use MediaWiki\Hook\BeforePageDisplayHook;
 use MediaWiki\Hook\InfoActionHook;
 use MediaWiki\MediaWikiServices;
+use MediaWiki\User\UserOptionsLookup;
 use OutputPage;
 use Skin;
 use Title;
@@ -55,6 +56,16 @@ class PageImages implements
 	 */
 	public const PROP_NAME_FREE = 'page_image_free';
 
+	/** @var UserOptionsLookup */
+	private $userOptionsLookup;
+
+	/**
+	 * @param UserOptionsLookup $userOptionsLookup
+	 */
+	public function __construct( UserOptionsLookup $userOptionsLookup ) {
+		$this->userOptionsLookup = $userOptionsLookup;
+	}
+
 	/**
 	 * Get property name used in page_props table. When a page image
 	 * is stored it will be stored under this property name on the corresponding
@@ -143,8 +154,7 @@ class PageImages implements
 			return;
 		}
 
-		$userOptionsLookup = MediaWikiServices::getInstance()->getUserOptionsLookup();
-		$thumbSetting = $userOptionsLookup->getOption( $context->getUser(), 'thumbsize' );
+		$thumbSetting = $this->userOptionsLookup->getOption( $context->getUser(), 'thumbsize' );
 		$thumbSize = $wgThumbLimits[$thumbSetting];
 
 		$thumb = $imageFile->transform( [ 'width' => $thumbSize ] );

tests/phpunit/Hooks/ParserFileProcessingHookHandlersTest.php

@@ -11,6 +11,7 @@ use Parser;
 use ParserOptions;
 use RepoGroup;
 use Title;
+use WANObjectCache;
 use Wikimedia\TestingAccessWrapper;
 
 /**
@@ -103,7 +104,7 @@ class ParserFileProcessingHookHandlersTest extends MediaWikiIntegrationTestCase
 
 	/**
 	 * @dataProvider provideDoParserAfterTidy
-	 * @covers \PageImages\Hooks\ParserFileProcessingHookHandlers::doParserAfterTidy
+	 * @covers \PageImages\Hooks\ParserFileProcessingHookHandlers::onParserAfterTidy
 	 */
 	public function testDoParserAfterTidy(
 		array $images,
@@ -113,7 +114,7 @@ class ParserFileProcessingHookHandlersTest extends MediaWikiIntegrationTestCase
 		$parser = $this->getParser( $images );
 		$html = $this->getHtml( array_keys( $images ) );
 		$handler = $this->getHandler( $images );
-		$handler->doParserAfterTidy( $parser, $html );
+		$handler->onParserAfterTidy( $parser, $html );
 		$properties = $parser->getOutput()->getPageProperties();
 
 		if ( $expectedFreeFileName === null ) {
@@ -172,7 +173,7 @@ class ParserFileProcessingHookHandlersTest extends MediaWikiIntegrationTestCase
 
 	/**
 	 * @dataProvider provideDoParserAfterTidy_lead
-	 * @covers \PageImages\Hooks\ParserFileProcessingHookHandlers::doParserAfterTidy
+	 * @covers \PageImages\Hooks\ParserFileProcessingHookHandlers::onParserAfterTidy
 	 */
 	public function testDoParserAfterTidy_lead( $leadOnly ) {
 		$this->setMwGlobals( 'wgPageImagesLeadSectionOnly', $leadOnly );
@@ -184,7 +185,7 @@ class ParserFileProcessingHookHandlersTest extends MediaWikiIntegrationTestCase
 		$parser = $this->getParser( $candidates );
 		$html = $this->getHtml( array_keys( $candidates ), 1 );
 		$handler = $this->getHandler( $candidates );
-		$handler->doParserAfterTidy( $parser, $html );
+		$handler->onParserAfterTidy( $parser, $html );
 		if ( $leadOnly ) {
 			$this->assertNull(
 				$parser->getOutput()->getPageProperty( PageImages::PROP_NAME_FREE ),
@@ -211,6 +212,7 @@ class ParserFileProcessingHookHandlersTest extends MediaWikiIntegrationTestCase
 	public function testGetScore( $image, $scoreFromTable, $position, $expected ) {
 		$mock = TestingAccessWrapper::newFromObject(
 			$this->getMockBuilder( ParserFileProcessingHookHandlers::class )
+				->setConstructorArgs( [ $this->getRepoGroup(), $this->createMock( WANObjectCache::class ) ] )
 				->onlyMethods( [ 'scoreFromTable', 'fetchFileMetadata', 'getRatio', 'getDenylist' ] )
 				->getMock()
 		);
@@ -277,7 +279,9 @@ class ParserFileProcessingHookHandlersTest extends MediaWikiIntegrationTestCase
 	 */
 	public function testScoreFromTable( array $scores, $value, $expected ) {
 		/** @var ParserFileProcessingHookHandlers $handlerWrapper */
-		$handlerWrapper = TestingAccessWrapper::newFromObject( new ParserFileProcessingHookHandlers );
+		$handlerWrapper = TestingAccessWrapper::newFromObject(
+			new ParserFileProcessingHookHandlers( $this->getRepoGroup(), $this->createMock( WANObjectCache::class ) )
+		);
 
 		$score = $handlerWrapper->scoreFromTable( $value, $scores );
 		$this->assertEquals( $expected, $score );
@@ -333,14 +337,9 @@ class ParserFileProcessingHookHandlersTest extends MediaWikiIntegrationTestCase
 	 * @covers \PageImages\Hooks\ParserFileProcessingHookHandlers::isImageFree
 	 */
 	public function testIsFreeImage( $fileName, $metadata, $expected ) {
-		$this->overrideMwServices( null, [
-			'RepoGroup' => function () {
-				return $this->getRepoGroup();
-			}
-		] );
-
 		$mock = TestingAccessWrapper::newFromObject(
 			$this->getMockBuilder( ParserFileProcessingHookHandlers::class )
+				->setConstructorArgs( [ $this->getRepoGroup(), $this->createMock( WANObjectCache::class ) ] )
 				->onlyMethods( [ 'fetchFileMetadata' ] )
 				->getMock()
 		);

tests/phpunit/PageImagesTest.php

@@ -26,7 +26,7 @@ class PageImagesTest extends MediaWikiIntegrationTestCase {
 	}
 
 	public function testConstructor() {
-		$pageImages = new PageImages();
+		$pageImages = new PageImages( $this->getServiceContainer()->getUserOptionsLookup() );
 		$this->assertInstanceOf( PageImages::class, $pageImages );
 	}
 
@@ -59,7 +59,8 @@ class PageImagesTest extends MediaWikiIntegrationTestCase {
 			->method( 'addMeta' );
 
 		$skinTemplate = new SkinTemplate();
-		( new PageImages() )->onBeforePageDisplay( $outputPage, $skinTemplate );
+		( new PageImages( $this->getServiceContainer()->getUserOptionsLookup() ) )
+			->onBeforePageDisplay( $outputPage, $skinTemplate );
 	}
 
 	public function testGivenNonExistingPageOnBeforePageDisplayDoesNotAddMeta() {
@@ -71,7 +72,8 @@ class PageImagesTest extends MediaWikiIntegrationTestCase {
 			->method( 'addMeta' );
 
 		$skinTemplate = new SkinTemplate();
-		( new PageImages() )->onBeforePageDisplay( $outputPage, $skinTemplate );
+		( new PageImages( $this->getServiceContainer()->getUserOptionsLookup() ) )
+			->onBeforePageDisplay( $outputPage, $skinTemplate );
 	}
 
 	public static function provideFallbacks() {
@@ -96,7 +98,8 @@ class PageImagesTest extends MediaWikiIntegrationTestCase {
 			->with( $this->equalTo( 'og:image' ), $this->equalTo( $expected ) );
 
 		$skinTemplate = new SkinTemplate();
-		( new PageImages() )->onBeforePageDisplay( $outputPage, $skinTemplate );
+		( new PageImages( $this->getServiceContainer()->getUserOptionsLookup() ) )
+			->onBeforePageDisplay( $outputPage, $skinTemplate );
 	}
 
 	/**