From 3e11a6940885c6a78f8da2d7cf58d4b0cfcef8df Mon Sep 17 00:00:00 2001 From: Marius Hoch Date: Tue, 10 Dec 2024 21:11:20 +0100 Subject: [PATCH] Prefetch terms of entities mentioned in edit summaries Prefetch terms of entities (limitted to items and properties) mentioned in edit summaries before displaying them in changes lists (recent changes, watchlist) and on edit history pages. As descriptions are also used (in "title" attributes) when rendering edit summaries on history pages, also prefetch descriptions. Bug: T292545 Change-Id: I20e7054ae31cebb0c98efe8030f71242d7677313 --- extension-repo.json | 4 +- .../DifferenceEngineViewHeaderHookHandler.php | 5 +- .../Hooks/LabelPrefetchHookHandler.php | 28 +++------ .../Hooks/PageHistoryPagerHookHandler.php | 41 +++---------- .../SummaryParsingPrefetchHelper.php | 60 ++++++++++--------- ...ferenceEngineViewHeaderHookHandlerTest.php | 2 +- .../Hooks/LabelPrefetchHookHandlerTest.php | 4 +- .../Hooks/LabelPrefetchHookHandlerTest.php | 6 +- .../LabelPrefetchHookHandlerTestBase.php | 7 +-- .../Hooks/PageHistoryPagerHookHandlerTest.php | 37 ++++++------ .../SummaryParsingPrefetchHelperTest.php | 42 ++++++------- 11 files changed, 99 insertions(+), 137 deletions(-) rename repo/includes/{FederatedProperties => Hooks}/SummaryParsingPrefetchHelper.php (50%) rename repo/tests/phpunit/includes/{FederatedProperties => }/Hooks/PageHistoryPagerHookHandlerTest.php (87%) rename repo/tests/phpunit/includes/{FederatedProperties => Hooks}/SummaryParsingPrefetchHelperTest.php (66%) diff --git a/extension-repo.json b/extension-repo.json index e96c7be8b59..513ae9ab697 100644 --- a/extension-repo.json +++ b/extension-repo.json @@ -1183,7 +1183,6 @@ "WikibaseRepo.EntityIdLookup", "WikibaseRepo.LanguageFallbackChainFactory", "WikibaseRepo.PrefetchingTermLookup", - "WikibaseRepo.Settings", "WikibaseRepo.TermBuffer" ] }, @@ -1226,8 +1225,7 @@ "services": [ "WikibaseRepo.LanguageFallbackChainFactory", "WikibaseRepo.LinkTargetEntityIdLookup", - "WikibaseRepo.PrefetchingTermLookup", - "WikibaseRepo.Settings" + "WikibaseRepo.PrefetchingTermLookup" ] }, "RecentChangeSave": { diff --git a/repo/includes/Hooks/DifferenceEngineViewHeaderHookHandler.php b/repo/includes/Hooks/DifferenceEngineViewHeaderHookHandler.php index d0b97a3d2e7..93bbf9dbf48 100644 --- a/repo/includes/Hooks/DifferenceEngineViewHeaderHookHandler.php +++ b/repo/includes/Hooks/DifferenceEngineViewHeaderHookHandler.php @@ -9,10 +9,9 @@ use Wikibase\Lib\LanguageFallbackChainFactory; use Wikibase\Lib\SettingsArray; use Wikibase\Lib\Store\LinkTargetEntityIdLookup; -use Wikibase\Repo\FederatedProperties\SummaryParsingPrefetchHelper; /** - * Hook for prefetching and handling federated properties before links are rendered. + * Hook for prefetching the terms of entities mentioned in edit summaries on diff pages. * * @license GPL-2.0-or-later */ @@ -85,7 +84,7 @@ public function onDifferenceEngineViewHeader( $differenceEngine ) { return; } - $this->summaryParsingPrefetcher->prefetchFederatedProperties( + $this->summaryParsingPrefetcher->prefetchTermsForMentionedEntities( [ $differenceEngine->getOldRevision(), $differenceEngine->getNewRevision() ], $this->languageFallbackChainFactory->newFromContext( $differenceEngine->getContext() )->getFetchLanguageCodes(), [ TermTypes::TYPE_LABEL ] diff --git a/repo/includes/Hooks/LabelPrefetchHookHandler.php b/repo/includes/Hooks/LabelPrefetchHookHandler.php index 2e0df5330e8..37405b0a963 100644 --- a/repo/includes/Hooks/LabelPrefetchHookHandler.php +++ b/repo/includes/Hooks/LabelPrefetchHookHandler.php @@ -9,11 +9,9 @@ use Wikibase\DataAccess\PrefetchingTermLookup; use Wikibase\DataModel\Services\Term\TermBuffer; use Wikibase\Lib\LanguageFallbackChainFactory; -use Wikibase\Lib\SettingsArray; use Wikibase\Lib\Store\EntityIdLookup; use Wikibase\Lib\Store\StorageException; use Wikibase\Lib\TermIndexEntry; -use Wikibase\Repo\FederatedProperties\SummaryParsingPrefetchHelper; use Wikimedia\Rdbms\IResultWrapper; /** @@ -53,15 +51,10 @@ class LabelPrefetchHookHandler implements ChangesListInitRowsHook { */ private $languageFallbackChainFactory; - /** - * @var bool - */ - private $federatedPropertiesEnabled; - /** * @var SummaryParsingPrefetchHelper */ - private $federatedPropertiesPrefetchingHelper; + private $summaryParsingPrefetchHelper; /** * @return self @@ -71,7 +64,6 @@ public static function factory( EntityIdLookup $entityIdLookup, LanguageFallbackChainFactory $languageFallbackChainFactory, PrefetchingTermLookup $prefetchingTermLookup, - SettingsArray $repoSettings, TermBuffer $termBuffer ): self { $termTypes = [ TermIndexEntry::TYPE_LABEL, TermIndexEntry::TYPE_DESCRIPTION ]; @@ -82,7 +74,6 @@ public static function factory( $titleFactory, $termTypes, $languageFallbackChainFactory, - $repoSettings->getSetting( 'federatedPropertiesEnabled' ), new SummaryParsingPrefetchHelper( $prefetchingTermLookup ) ); } @@ -93,7 +84,6 @@ public static function factory( * @param TitleFactory $titleFactory * @param string[] $termTypes * @param LanguageFallbackChainFactory $languageFallbackChainFactory - * @param bool $federatedPropertiesEnabled * @param SummaryParsingPrefetchHelper $summaryParsingPrefetchHelper */ public function __construct( @@ -102,7 +92,6 @@ public function __construct( TitleFactory $titleFactory, array $termTypes, LanguageFallbackChainFactory $languageFallbackChainFactory, - bool $federatedPropertiesEnabled, SummaryParsingPrefetchHelper $summaryParsingPrefetchHelper ) { $this->buffer = $buffer; @@ -110,8 +99,7 @@ public function __construct( $this->titleFactory = $titleFactory; $this->termTypes = $termTypes; $this->languageFallbackChainFactory = $languageFallbackChainFactory; - $this->federatedPropertiesEnabled = $federatedPropertiesEnabled; - $this->federatedPropertiesPrefetchingHelper = $summaryParsingPrefetchHelper; + $this->summaryParsingPrefetchHelper = $summaryParsingPrefetchHelper; } /** @@ -126,13 +114,11 @@ public function onChangesListInitRows( $list, $rows ): void { ->getFetchLanguageCodes(); $this->buffer->prefetchTerms( $entityIds, $this->termTypes, $languageCodes ); - if ( $this->federatedPropertiesEnabled ) { - $this->federatedPropertiesPrefetchingHelper->prefetchFederatedProperties( - $rows, - $languageCodes, - $this->termTypes - ); - } + $this->summaryParsingPrefetchHelper->prefetchTermsForMentionedEntities( + $rows, + $languageCodes, + $this->termTypes + ); } catch ( StorageException $ex ) { wfLogWarning( __METHOD__ . ': ' . $ex->getMessage() ); diff --git a/repo/includes/Hooks/PageHistoryPagerHookHandler.php b/repo/includes/Hooks/PageHistoryPagerHookHandler.php index 38d941c763d..2de266b3e2a 100644 --- a/repo/includes/Hooks/PageHistoryPagerHookHandler.php +++ b/repo/includes/Hooks/PageHistoryPagerHookHandler.php @@ -9,62 +9,40 @@ use Wikibase\DataAccess\PrefetchingTermLookup; use Wikibase\DataModel\Term\TermTypes; use Wikibase\Lib\LanguageFallbackChainFactory; -use Wikibase\Lib\SettingsArray; use Wikibase\Lib\Store\LinkTargetEntityIdLookup; -use Wikibase\Repo\FederatedProperties\SummaryParsingPrefetchHelper; use Wikimedia\Rdbms\IResultWrapper; //phpcs:disable MediaWiki.NamingConventions.LowerCamelFunctionsName.FunctionName /** * Hook handler for prefetching on history pages. * - * Currently only used when federated properties are enabled. - * * @license GPL-2.0-or-later * @author Tobias Andersson */ class PageHistoryPagerHookHandler implements PageHistoryPager__doBatchLookupsHook { - /** @var bool */ - private $federatedPropertiesEnabled; - - /** @var LinkTargetEntityIdLookup */ - private $linkTargetEntityIdLookup; + private LinkTargetEntityIdLookup $linkTargetEntityIdLookup; - /** @var LanguageFallbackChainFactory */ - private $languageFallbackChainFactory; + private LanguageFallbackChainFactory $languageFallbackChainFactory; - /** @var SummaryParsingPrefetchHelper|null */ - private $federatedPropertiesPrefetchHelper; + private SummaryParsingPrefetchHelper $summaryParsingPrefetchHelper; - /** - * @param bool $federatedPropertiesEnabled - * @param PrefetchingTermLookup $prefetchingLookup - * @param LinkTargetEntityIdLookup $linkTargetEntityIdLookup - * @param LanguageFallbackChainFactory $languageFallbackChainFactory - */ public function __construct( - bool $federatedPropertiesEnabled, PrefetchingTermLookup $prefetchingLookup, LinkTargetEntityIdLookup $linkTargetEntityIdLookup, LanguageFallbackChainFactory $languageFallbackChainFactory ) { - $this->federatedPropertiesEnabled = $federatedPropertiesEnabled; $this->linkTargetEntityIdLookup = $linkTargetEntityIdLookup; $this->languageFallbackChainFactory = $languageFallbackChainFactory; - if ( $federatedPropertiesEnabled ) { - $this->federatedPropertiesPrefetchHelper = new SummaryParsingPrefetchHelper( $prefetchingLookup ); - } + $this->summaryParsingPrefetchHelper = new SummaryParsingPrefetchHelper( $prefetchingLookup ); } public static function factory( LanguageFallbackChainFactory $languageFallbackChainFactory, LinkTargetEntityIdLookup $linkTargetEntityIdLookup, - PrefetchingTermLookup $prefetchingTermLookup, - SettingsArray $repoSettings + PrefetchingTermLookup $prefetchingTermLookup ): self { return new self( - $repoSettings->getSetting( 'federatedPropertiesEnabled' ), $prefetchingTermLookup, $linkTargetEntityIdLookup, $languageFallbackChainFactory, @@ -76,21 +54,18 @@ public static function factory( * @param IResultWrapper $result */ public function onPageHistoryPager__doBatchLookups( $pager, $result ) { - if ( !$this->federatedPropertiesEnabled ) { - return; - } - $entityId = $this->linkTargetEntityIdLookup->getEntityId( $pager->getTitle() ); if ( $entityId === null ) { + // XXX: This means we only prefetch when showing the edit history of an entity. return; } $languageFallbackChain = $this->languageFallbackChainFactory->newFromContext( $pager->getContext() ); - $this->federatedPropertiesPrefetchHelper->prefetchFederatedProperties( + $this->summaryParsingPrefetchHelper->prefetchTermsForMentionedEntities( $result, $languageFallbackChain->getFetchLanguageCodes(), - [ TermTypes::TYPE_LABEL ] + [ TermTypes::TYPE_LABEL, TermTypes::TYPE_DESCRIPTION ] ); } } diff --git a/repo/includes/FederatedProperties/SummaryParsingPrefetchHelper.php b/repo/includes/Hooks/SummaryParsingPrefetchHelper.php similarity index 50% rename from repo/includes/FederatedProperties/SummaryParsingPrefetchHelper.php rename to repo/includes/Hooks/SummaryParsingPrefetchHelper.php index 3b886649e09..ae6bccaebcf 100644 --- a/repo/includes/FederatedProperties/SummaryParsingPrefetchHelper.php +++ b/repo/includes/Hooks/SummaryParsingPrefetchHelper.php @@ -1,49 +1,52 @@ prefetchingLookup = $prefetchingLookup; + public function __construct( + TermBuffer $termBuffer + ) { + $this->termBuffer = $termBuffer; + $this->entityIdParser = new BasicEntityIdParser(); } /** - * Matching links to properties in in edit summaries, such as "[[Property:P123]]" or "[[wdbeta:Special:EntityPage/P123]]" + * Matching links to properties in in edit summaries, such as "[[Q23]]", "[[Property:P123]]" + * or "[[wdbeta:Special:EntityPage/P123]]". */ - public const PROPERTY_SUMMARY_REGEXP = '/\[\[(\S+)(P[1-9]\d*)\]\]/'; + public const ENTITY_ID_SUMMARY_REGEXP = '/\[\[(\S*)([PQ][1-9]\d*)\]\]/'; /** * @param IResultWrapper|\stdClass[]|RevisionRecord[] $rows * @param array $languageCodes * @param array $termTypes */ - public function prefetchFederatedProperties( $rows, array $languageCodes, array $termTypes ): void { - $resultProperties = $this->extractSummaryProperties( $rows ); - if ( !$resultProperties ) { + public function prefetchTermsForMentionedEntities( $rows, array $languageCodes, array $termTypes ): void { + $mentionedEntityIds = $this->extractSummaryMentions( $rows ); + if ( !$mentionedEntityIds ) { return; } - $this->prefetchingLookup->prefetchTerms( - $resultProperties, + $this->termBuffer->prefetchTerms( + $mentionedEntityIds, $termTypes, $languageCodes ); @@ -51,10 +54,10 @@ public function prefetchFederatedProperties( $rows, array $languageCodes, array /** * @param IResultWrapper|\stdClass[]|RevisionRecord[] $result - * @return PropertyId[] + * @return EntityId[] */ - public function extractSummaryProperties( $result ): array { - $propertyIds = []; + public function extractSummaryMentions( $result ): array { + $entityIds = []; foreach ( $result as $revisionRow ) { $comment = $this->getCommentText( $revisionRow ); if ( $comment === null ) { @@ -62,14 +65,17 @@ public function extractSummaryProperties( $result ): array { } $matches = []; - preg_match( self::PROPERTY_SUMMARY_REGEXP, $comment, $matches ); - if ( count( $matches ) === 3 ) { - $propertyId = $matches[2]; - // TODO: Change to FederatedPropertyId when this functionality is supported in Feddy Props v2 - $propertyIds[] = new NumericPropertyId( $propertyId ); + if ( !preg_match_all( self::ENTITY_ID_SUMMARY_REGEXP, $comment, $matches, PREG_PATTERN_ORDER ) ) { + continue; + } + foreach ( $matches[2] as $match ) { + try { + $entityIds[] = $this->entityIdParser->parse( $match ); + } catch ( EntityIdParsingException $ex ) { + } } } - return $propertyIds; + return $entityIds; } /** diff --git a/repo/tests/phpunit/includes/FederatedProperties/Hooks/DifferenceEngineViewHeaderHookHandlerTest.php b/repo/tests/phpunit/includes/FederatedProperties/Hooks/DifferenceEngineViewHeaderHookHandlerTest.php index b709b1fbad8..035aa46209c 100644 --- a/repo/tests/phpunit/includes/FederatedProperties/Hooks/DifferenceEngineViewHeaderHookHandlerTest.php +++ b/repo/tests/phpunit/includes/FederatedProperties/Hooks/DifferenceEngineViewHeaderHookHandlerTest.php @@ -21,8 +21,8 @@ use Wikibase\Lib\LanguageWithConversion; use Wikibase\Lib\Store\LinkTargetEntityIdLookup; use Wikibase\Lib\TermLanguageFallbackChain; -use Wikibase\Repo\FederatedProperties\SummaryParsingPrefetchHelper; use Wikibase\Repo\Hooks\DifferenceEngineViewHeaderHookHandler; +use Wikibase\Repo\Hooks\SummaryParsingPrefetchHelper; /** * @covers \Wikibase\Repo\Hooks\DifferenceEngineViewHeaderHookHandler diff --git a/repo/tests/phpunit/includes/FederatedProperties/Hooks/LabelPrefetchHookHandlerTest.php b/repo/tests/phpunit/includes/FederatedProperties/Hooks/LabelPrefetchHookHandlerTest.php index 7eb6765d841..3a77f74a197 100644 --- a/repo/tests/phpunit/includes/FederatedProperties/Hooks/LabelPrefetchHookHandlerTest.php +++ b/repo/tests/phpunit/includes/FederatedProperties/Hooks/LabelPrefetchHookHandlerTest.php @@ -58,7 +58,6 @@ public function testDoChangesListInitRows() { $itemTwo->getStatements()->addStatement( new Statement( new PropertyNoValueSnak( 3 ) ) ); $itemThreeNotFoundId = new ItemId( 'Q3' ); - $itemThreeNotFound = null; $expectedItemIds = [ $itemOneId, @@ -85,8 +84,7 @@ public function testDoChangesListInitRows() { $this->getPrefetchTermsCallback( $expectedItemIds, $expectedTermTypes, $expectedLanguageCodes ), $expectedTermTypes, $expectedLanguageCodes, - $prefetchingTermLookup, - true + $prefetchingTermLookup ); /** @var ChangesList $changesList */ diff --git a/repo/tests/phpunit/includes/Hooks/LabelPrefetchHookHandlerTest.php b/repo/tests/phpunit/includes/Hooks/LabelPrefetchHookHandlerTest.php index bef186928de..ea4899376a0 100644 --- a/repo/tests/phpunit/includes/Hooks/LabelPrefetchHookHandlerTest.php +++ b/repo/tests/phpunit/includes/Hooks/LabelPrefetchHookHandlerTest.php @@ -21,9 +21,9 @@ class LabelPrefetchHookHandlerTest extends LabelPrefetchHookHandlerTestBase { public function testDoChangesListInitRows() { $rows = [ - (object)[ 'rc_namespace' => NS_MAIN, 'rc_title' => 'XYZ' ], - (object)[ 'rc_namespace' => NS_MAIN, 'rc_title' => 'Q23' ], - (object)[ 'rc_namespace' => NS_MAIN, 'rc_title' => 'P55' ], + (object)[ 'rc_namespace' => NS_MAIN, 'rc_title' => 'XYZ', 'rc_comment_text' => '' ], + (object)[ 'rc_namespace' => NS_MAIN, 'rc_title' => 'Q23', 'rc_comment_text' => 'foo' ], + (object)[ 'rc_namespace' => NS_MAIN, 'rc_title' => 'P55', 'rc_comment_text' => '' ], ]; $expectedTermTypes = [ 'label', 'description' ]; diff --git a/repo/tests/phpunit/includes/Hooks/LabelPrefetchHookHandlerTestBase.php b/repo/tests/phpunit/includes/Hooks/LabelPrefetchHookHandlerTestBase.php index 6bc6ffe8e76..d951c2728ad 100644 --- a/repo/tests/phpunit/includes/Hooks/LabelPrefetchHookHandlerTestBase.php +++ b/repo/tests/phpunit/includes/Hooks/LabelPrefetchHookHandlerTestBase.php @@ -13,8 +13,8 @@ use Wikibase\Lib\LanguageFallbackChainFactory; use Wikibase\Lib\Store\EntityIdLookup; use Wikibase\Lib\TermLanguageFallbackChain; -use Wikibase\Repo\FederatedProperties\SummaryParsingPrefetchHelper; use Wikibase\Repo\Hooks\LabelPrefetchHookHandler; +use Wikibase\Repo\Hooks\SummaryParsingPrefetchHelper; /** * @covers \Wikibase\Repo\Hooks\LabelPrefetchHookHandler @@ -54,15 +54,13 @@ public function titlesToIds( array $titles ) { * @param string[] $termTypes * @param string[] $languageCodes * @param PrefetchingTermLookup|null $prefetchingTermLookup - * @param bool $federatedPropertiesEnabled * @return LabelPrefetchHookHandler */ protected function getLabelPrefetchHookHandlers( $prefetchTerms, array $termTypes, array $languageCodes, - PrefetchingTermLookup $prefetchingTermLookup = null, - bool $federatedPropertiesEnabled = false + PrefetchingTermLookup $prefetchingTermLookup = null ) { $prefetchingTermLookup ??= $this->createMock( PrefetchingTermLookup::class ); @@ -93,7 +91,6 @@ protected function getLabelPrefetchHookHandlers( $titleFactory, $termTypes, $fallbackChainFactory, - $federatedPropertiesEnabled, new SummaryParsingPrefetchHelper( $prefetchingTermLookup ) ); } diff --git a/repo/tests/phpunit/includes/FederatedProperties/Hooks/PageHistoryPagerHookHandlerTest.php b/repo/tests/phpunit/includes/Hooks/PageHistoryPagerHookHandlerTest.php similarity index 87% rename from repo/tests/phpunit/includes/FederatedProperties/Hooks/PageHistoryPagerHookHandlerTest.php rename to repo/tests/phpunit/includes/Hooks/PageHistoryPagerHookHandlerTest.php index 053d4fa4fde..e195dc08c1e 100644 --- a/repo/tests/phpunit/includes/FederatedProperties/Hooks/PageHistoryPagerHookHandlerTest.php +++ b/repo/tests/phpunit/includes/Hooks/PageHistoryPagerHookHandlerTest.php @@ -2,7 +2,7 @@ declare( strict_types=1 ); -namespace Wikibase\Repo\Tests\FederatedProperties\Hooks; +namespace Wikibase\Repo\Tests\Hooks; use MediaWiki\Context\IContextSource; use MediaWiki\Pager\HistoryPager; @@ -12,7 +12,6 @@ use Wikibase\DataModel\Entity\Item; use Wikibase\DataModel\Entity\ItemId; use Wikibase\DataModel\Entity\NumericPropertyId; -use Wikibase\DataModel\Services\Lookup\EntityLookup; use Wikibase\DataModel\Snak\PropertyNoValueSnak; use Wikibase\DataModel\Statement\Statement; use Wikibase\DataModel\Statement\StatementList; @@ -35,9 +34,6 @@ */ class PageHistoryPagerHookHandlerTest extends TestCase { - /** @var EntityLookup */ - private $entityLookup; - /** @var PrefetchingTermLookup */ private $prefetchingLookup; @@ -78,7 +74,6 @@ protected function setUp(): void { $this->prefetchingLookup = $this->createMock( PrefetchingTermLookup::class ); $this->languageFallbackChainFactory = $this->createMock( LanguageFallbackChainFactory::class ); - $this->entityLookup = $this->createMock( EntityLookup::class ); $this->linkTargetEntityIdLookup = $this->createMock( LinkTargetEntityIdLookup::class ); $stubContentLanguages = $this->createStub( ContentLanguages::class ); @@ -101,29 +96,30 @@ protected function setUp(): void { ->willReturn( $this->languageFallback ); } - private function getHookHandler( bool $federatedPropertiesEnabled ) { + private function getHookHandler() { return new PageHistoryPagerHookHandler( - $federatedPropertiesEnabled, $this->prefetchingLookup, $this->linkTargetEntityIdLookup, $this->languageFallbackChainFactory ); } - public function setupResultWithSummaries( StatementList $statementList ) { + private function setupResultWithSummaries( StatementList $statementList ) { $availableProperties = array_map( function( $snak ) { return $snak->getPropertyId(); }, $statementList->getAllSnaks() ); - $summaries = array_map( function ( $prop ) { + $i = 0; + $summaries = array_map( function ( $prop ) use ( &$i ) { + $i++; $object = (object)[ - 'rev_comment_text' => "/* wbsetclaim-update:1||1 */ [[Property:{$prop->getSerialization()}]]: asdfasdasfas", + 'rev_comment_text' => "/* wbsetclaim-update:1||1 */ [[Property:{$prop->getSerialization()}]]: foo [[Q$i]] bar.", ]; return $object; }, $availableProperties ); $valid = []; - for ( $i = 1; $i < count( $summaries ); $i++ ) { + for ( $i = 0; $i < count( $summaries ); $i++ ) { $valid[] = true; } $valid[] = false; @@ -154,12 +150,19 @@ public function testDoBatchLookups() { $this->prefetchingLookup->expects( $this->once() ) ->method( 'prefetchTerms' ) ->with( - [ new NumericPropertyId( "P666" ), new NumericPropertyId( "P12345" ) ], - [ TermTypes::TYPE_LABEL ], + [ + new NumericPropertyId( "P666" ), + new ItemId( 'Q1' ), + new NumericPropertyId( "P12345" ), + new ItemId( 'Q2' ), + new NumericPropertyId( "P12345" ), + new ItemId( 'Q3' ), + ], + [ TermTypes::TYPE_LABEL, TermTypes::TYPE_DESCRIPTION ], $this->languageCodes ); - $handler = $this->getHookHandler( true ); + $handler = $this->getHookHandler(); $handler->onPageHistoryPager__doBatchLookups( $this->pager, $this->resultWrapper ); } @@ -179,7 +182,7 @@ public function testDontPrefetchEmptySummary() { $this->prefetchingLookup->expects( $this->never() ) ->method( 'prefetchTerms' ); - $handler = $this->getHookHandler( true ); + $handler = $this->getHookHandler(); $handler->onPageHistoryPager__doBatchLookups( $this->pager, $this->resultWrapper ); } @@ -197,7 +200,7 @@ public function testShouldSkipIfNotEntityId() { $this->prefetchingLookup->expects( $this->never() ) ->method( 'prefetchTerms' ); - $handler = $this->getHookHandler( true ); + $handler = $this->getHookHandler(); $handler->onPageHistoryPager__doBatchLookups( $this->pager, $this->resultWrapper ); } diff --git a/repo/tests/phpunit/includes/FederatedProperties/SummaryParsingPrefetchHelperTest.php b/repo/tests/phpunit/includes/Hooks/SummaryParsingPrefetchHelperTest.php similarity index 66% rename from repo/tests/phpunit/includes/FederatedProperties/SummaryParsingPrefetchHelperTest.php rename to repo/tests/phpunit/includes/Hooks/SummaryParsingPrefetchHelperTest.php index 09f60316158..4ff1ea2f56c 100644 --- a/repo/tests/phpunit/includes/FederatedProperties/SummaryParsingPrefetchHelperTest.php +++ b/repo/tests/phpunit/includes/Hooks/SummaryParsingPrefetchHelperTest.php @@ -1,17 +1,17 @@ prefetchingLookup ); - $expectedPropertyIds = []; - foreach ( $expectedProperties as $propertyString ) { - $expectedPropertyIds[] = new NumericPropertyId( $propertyString ); + $expectedEntityIds = []; + $entityIdParser = new BasicEntityIdParser(); + foreach ( $expected as $idSerialization ) { + $expectedEntityIds[] = $entityIdParser->parse( $idSerialization ); } - $this->prefetchingLookup->expects( empty( $expectedPropertyIds ) ? $this->never() : $this->once() ) + $this->prefetchingLookup->expects( empty( $expectedEntityIds ) ? $this->never() : $this->once() ) ->method( 'prefetchTerms' ) ->with( - $expectedPropertyIds, + $expectedEntityIds, [ TermTypes::TYPE_LABEL ], [ 'en' ] ); - $helper->prefetchFederatedProperties( $rows, [ 'en' ], [ TermTypes::TYPE_LABEL ] ); + $helper->prefetchTermsForMentionedEntities( $rows, [ 'en' ], [ TermTypes::TYPE_LABEL ] ); } /** * @dataProvider rowDataProvider */ - public function testShouldExtractProperties( callable $rowsFactory, array $expectedProperties ) { + public function testShouldExtractProperties( callable $rowsFactory, array $expected ) { $rows = $rowsFactory( $this ); $helper = new SummaryParsingPrefetchHelper( $this->prefetchingLookup ); - $actualOutput = $helper->extractSummaryProperties( $rows ); + $actualOutput = $helper->extractSummaryMentions( $rows ); - $this->assertSameSize( $expectedProperties, $actualOutput ); + $this->assertSameSize( $expected, $actualOutput ); $stringOutput = array_map( function ( $propId ) { return $propId->getSerialization(); }, $actualOutput ); - $this->assertSame( sort( $expectedProperties ), sort( $stringOutput ) ); + $this->assertSame( sort( $expected ), sort( $stringOutput ) ); } public static function rowDataProvider() { return [ - 'Property:P1' => [ + 'Property:P31' => [ fn () => [ (object)[ 'rev_comment_text' => '[[Property:P31]]' ] ], [ 'P31' ], ], + 'Links to main namespace' => [ + fn () => [ (object)[ 'rev_comment_text' => '[[P11]] [[Q22]] [[P33]]' ] ], + [ 'P11', 'Q22', 'P33' ], + ], 'wdbeta:Special:EntityPage/P123' => [ fn () => [ (object)[ 'rev_comment_text' => '[[wdbeta:Special:EntityPage/P123]]' ], @@ -83,7 +88,6 @@ public static function rowDataProvider() { 'Some other comment not parsed as link' => [ fn () => [ (object)[ 'rev_comment_text' => 'Great update /P14 stockholm' ], - (object)[ 'rc_comment_text' => '[[P31]]' ], (object)[ 'rc_comment_text' => 'P31]]' ], (object)[ 'rc_comment_text' => '[P31:P31]' ], ], @@ -97,10 +101,6 @@ public static function rowDataProvider() { fn ( self $self ) => [ $self->mockRevisionRecord( 'something [[Property:P31]]' ) ], [ 'P31' ], ], - 'RevisionRecord no match' => [ - fn ( self $self ) => [ $self->mockRevisionRecord( 'something [[P31]]' ) ], - [], - ], 'null' => [ fn () => [ (object)[ 'rc_comment_text' => null ] ], [] ], ]; }