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
wikimedia · Dec 12, 2024 · 3e11a69 · 3e11a69
1 parent 4855add
commit 3e11a69
Show file tree

Hide file tree

Showing 11 changed files with 99 additions and 137 deletions.
diff --git 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
@@ -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
@@ -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,16 +92,14 @@ public function __construct(
 		TitleFactory $titleFactory,
 		array $termTypes,
 		LanguageFallbackChainFactory $languageFallbackChainFactory,
-		bool $federatedPropertiesEnabled,
 		SummaryParsingPrefetchHelper $summaryParsingPrefetchHelper
 	) {
 		$this->buffer = $buffer;
 		$this->idLookup = $idLookup;
 		$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
@@ -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/...operties/SummaryParsingPrefetchHelper.php → ...es/Hooks/SummaryParsingPrefetchHelper.php b/...operties/SummaryParsingPrefetchHelper.php → ...es/Hooks/SummaryParsingPrefetchHelper.php
@@ -1,75 +1,81 @@
 <?php
 declare( strict_types = 1 );
 
-namespace Wikibase\Repo\FederatedProperties;
+namespace Wikibase\Repo\Hooks;
 
 use LogicException;
 use MediaWiki\Revision\RevisionRecord;
-use Wikibase\DataAccess\PrefetchingTermLookup;
-use Wikibase\DataModel\Entity\NumericPropertyId;
-use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\DataModel\Entity\BasicEntityIdParser;
+use Wikibase\DataModel\Entity\EntityId;
+use Wikibase\DataModel\Entity\EntityIdParsingException;
+use Wikibase\DataModel\Services\Term\TermBuffer;
 use Wikimedia\Rdbms\IResultWrapper;
 
 /**
- * A helper class for parsing and prefetching properties from summaries for federated properties
+ * A helper class for parsing and prefetching terms of entities mentioned in edit summaries.
  *
  * @license GPL-2.0-or-later
  */
 class SummaryParsingPrefetchHelper {
 
-	/**
-	 * @var PrefetchingTermLookup
-	 */
-	private $prefetchingLookup;
+	private TermBuffer $termBuffer;
+	private BasicEntityIdParser $entityIdParser;
 
-	public function __construct( PrefetchingTermLookup $prefetchingLookup ) {
-		$this->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
 		);
 	}
 
 	/**
 	 * @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 ) {
 				continue;
 			}
 
 			$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/.../phpunit/includes/FederatedProperties/Hooks/DifferenceEngineViewHeaderHookHandlerTest.php b/.../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
@@ -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
@@ -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' ];