Merge "Use LinkBatch in AffectedPagesFinder"

wikimedia · Nov 4, 2020 · fd45e5a · fd45e5a
2 parents 2370710 + 56685d6
commit fd45e5a
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 18 deletions.
diff --git a/client/includes/Changes/AffectedPagesFinder.php b/client/includes/Changes/AffectedPagesFinder.php
@@ -1,10 +1,14 @@
 <?php
 
+declare( strict_types = 1 );
+
 namespace Wikibase\Client\Changes;
 
 use ArrayIterator;
 use InvalidArgumentException;
+use MediaWiki\Cache\LinkBatchFactory;
 use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
 use Title;
 use TitleFactory;
 use Traversable;
@@ -13,7 +17,6 @@
 use Wikibase\Client\Usage\PageEntityUsages;
 use Wikibase\Client\Usage\UsageAspectTransformer;
 use Wikibase\Client\Usage\UsageLookup;
-use Wikibase\Client\WikibaseClient;
 use Wikibase\DataModel\Entity\EntityId;
 use Wikibase\Lib\Changes\Change;
 use Wikibase\Lib\Changes\EntityChange;
@@ -36,25 +39,30 @@ class AffectedPagesFinder {
 	 */
 	private $titleFactory;
 
+	/** @var LinkBatchFactory */
+	private $linkBatchFactory;
+
 	/**
 	 * @var string
 	 */
 	private $siteId;
 
 	/**
-	 * @var bool
+	 * @var LoggerInterface
 	 */
-	private $checkPageExistence;
+	private $logger;
 
 	/**
-	 * @var LoggerInterface
+	 * @var bool
 	 */
-	private $logger;
+	private $checkPageExistence;
 
 	/**
 	 * @param UsageLookup $usageLookup
 	 * @param TitleFactory $titleFactory
+	 * @param LinkBatchFactory $linkBatchFactory
 	 * @param string $siteId
+	 * @param LoggerInterface|null $logger
 	 * @param bool $checkPageExistence To disable slow filtering that is not relevant in test
 	 *  scenarios. Not meant to be used in production!
 	 *
@@ -63,23 +71,17 @@ class AffectedPagesFinder {
 	public function __construct(
 		UsageLookup $usageLookup,
 		TitleFactory $titleFactory,
-		$siteId,
-		$checkPageExistence = true
+		LinkBatchFactory $linkBatchFactory,
+		string $siteId,
+		?LoggerInterface $logger = null,
+		bool $checkPageExistence = true
 	) {
-		if ( !is_string( $siteId ) ) {
-			throw new InvalidArgumentException( '$siteId must be a string' );
-		}
-
-		if ( !is_bool( $checkPageExistence ) ) {
-			throw new InvalidArgumentException( '$checkPageExistence must be a boolean' );
-		}
-
 		$this->usageLookup = $usageLookup;
 		$this->titleFactory = $titleFactory;
+		$this->linkBatchFactory = $linkBatchFactory;
 		$this->siteId = $siteId;
+		$this->logger = $logger ?: new NullLogger();
 		$this->checkPageExistence = $checkPageExistence;
-		// TODO inject me
-		$this->logger = WikibaseClient::getDefaultInstance()->getLogger();
 	}
 
 	/**
@@ -335,6 +337,9 @@ private function makeVirtualUsages( array $titles, EntityId $entityId, array $as
 			$usagesForItem[] = new EntityUsage( $entityId, $aspect, $modifier );
 		}
 
+		// bulk-load the page IDs into the LinkCache
+		$this->linkBatchFactory->newLinkBatch( $titles )->execute();
+
 		$usagesPerPage = [];
 		foreach ( $titles as $title ) {
 			$pageId = $title->getArticleID();

diff --git a/client/includes/WikibaseClient.php b/client/includes/WikibaseClient.php
@@ -1123,7 +1123,9 @@ private function getAffectedPagesFinder(): AffectedPagesFinder {
 		return new AffectedPagesFinder(
 			$this->getStore()->getUsageLookup(),
 			new TitleFactory(),
-			$this->settings->getSetting( 'siteGlobalID' )
+			MediaWikiServices::getInstance()->getLinkBatchFactory(),
+			$this->settings->getSetting( 'siteGlobalID' ),
+			$this->getLogger()
 		);
 	}
 

diff --git a/client/tests/phpunit/integration/includes/Changes/AffectedPagesFinderTest.php b/client/tests/phpunit/integration/includes/Changes/AffectedPagesFinderTest.php
@@ -1,10 +1,14 @@
 <?php
 
+declare( strict_types = 1 );
+
 namespace Wikibase\Client\Tests\Integration\Changes;
 
 use ArrayIterator;
 use DataValues\DataValue;
 use DataValues\StringValue;
+use LinkBatch;
+use MediaWiki\Cache\LinkBatchFactory;
 use Title;
 use TitleFactory;
 use Traversable;
@@ -65,6 +69,18 @@ private function getTitleFactory() {
 		return $titleFactory;
 	}
 
+	private function getLinkBatchFactory(): LinkBatchFactory {
+		$linkBatch = $this->createMock( LinkBatch::class );
+		$linkBatch->method( 'execute' )
+			->willReturn( null );
+
+		$linkBatchFactory = $this->createMock( LinkBatchFactory::class );
+		$linkBatchFactory->method( 'newLinkBatch' )
+			->willReturn( $linkBatch );
+
+		return $linkBatchFactory;
+	}
+
 	private function getAffectedPagesFinder( array $usage, array $expectedAspects ) {
 		$usageLookup = $this->createMock( UsageLookup::class );
 
@@ -76,7 +92,9 @@ private function getAffectedPagesFinder( array $usage, array $expectedAspects )
 		$affectedPagesFinder = new AffectedPagesFinder(
 			$usageLookup,
 			$this->getTitleFactory(),
+			$this->getLinkBatchFactory(),
 			'enwiki',
+			null,
 			false
 		);
 
@@ -513,7 +531,9 @@ public function testGetAffectedUsagesByPage_withDeletedPage() {
 		$affectedPagesFinder = new AffectedPagesFinder(
 			$this->getSiteLinkUsageLookup( $pageTitle ),
 			new TitleFactory(),
+			$this->getLinkBatchFactory(),
 			'enwiki',
+			null,
 			false
 		);
 

diff --git a/client/tests/phpunit/integration/includes/Changes/ChangeHandlerTest.php b/client/tests/phpunit/integration/includes/Changes/ChangeHandlerTest.php
@@ -4,6 +4,7 @@
 
 use ArrayIterator;
 use InvalidArgumentException;
+use MediaWiki\MediaWikiServices;
 use MediaWikiIntegrationTestCase;
 use Psr\Log\NullLogger;
 use SiteLookup;
@@ -44,7 +45,9 @@ private function getAffectedPagesFinder( UsageLookup $usageLookup, TitleFactory
 		return new AffectedPagesFinder(
 			$usageLookup,
 			$titleFactory,
+			MediaWikiServices::getInstance()->getLinkBatchFactory(),
 			'enwiki',
+			null,
 			false
 		);
 	}