diff --git a/client/WikibaseClient.ServiceWiring.php b/client/WikibaseClient.ServiceWiring.php index 0aaba20f640..2970931fdb3 100644 --- a/client/WikibaseClient.ServiceWiring.php +++ b/client/WikibaseClient.ServiceWiring.php @@ -657,7 +657,8 @@ function ( EntityNamespaceLookup $nsLookup, DatabaseEntitySource $source ): Enti WikibaseClient::getUsageAccumulatorFactory( $services ), $settings->getSetting( 'siteGlobalID' ), $services->getRevisionLookup(), - WikibaseClient::getLogger( $services ) + WikibaseClient::getTermLookup( $services ), + WikibaseClient::getLogger( $services ), ); }, diff --git a/client/includes/ClientHooks.php b/client/includes/ClientHooks.php index 8489580d962..97ff9ddd3b0 100644 --- a/client/includes/ClientHooks.php +++ b/client/includes/ClientHooks.php @@ -182,20 +182,18 @@ public static function onCirrusSearchAddQueryFeatures( * * @return bool Always true. */ - public static function onSkinAfterBottomScripts( Skin $skin, &$html ) { + public static function onSkinAfterBottomScripts( Skin $skin, string &$html ) { $services = MediaWikiServices::getInstance(); $enabledNamespaces = WikibaseClient::getSettings( $services ) ->getSetting( 'pageSchemaNamespaces' ); $generator = new LinkedDataSchemaGenerator( - $services->getContentLanguage(), WikibaseClient::getRepoLinker( $services ), - WikibaseClient::getTermLookup( $services ), ); - $out = $skin->getOutput(); - $entityId = self::parseEntityId( $out->getProperty( 'wikibase_item' ) ); - $title = $out->getTitle(); + $outputPage = $skin->getOutput(); + $entityId = self::parseEntityId( $outputPage->getProperty( 'wikibase_item' ) ); + $title = $outputPage->getTitle(); if ( !$entityId || !$title || @@ -205,14 +203,15 @@ public static function onSkinAfterBottomScripts( Skin $skin, &$html ) { return true; } - $revisionTimestamp = $out->getRevisionTimestamp(); - $firstRevisionTimestamp = $out->getProperty( 'first_revision_timestamp' ); - + $revisionTimestamp = $outputPage->getRevisionTimestamp(); + $firstRevisionTimestamp = $outputPage->getProperty( 'first_revision_timestamp' ); + $description = $outputPage->getProperty( 'wikibase_item_description' ); $html .= $generator->createSchemaElement( $title, $revisionTimestamp, $firstRevisionTimestamp, - $entityId + $entityId, + $description ); return true; diff --git a/client/includes/Hooks/LinkedDataSchemaGenerator.php b/client/includes/Hooks/LinkedDataSchemaGenerator.php index c9dfc864db4..bb692fb6921 100644 --- a/client/includes/Hooks/LinkedDataSchemaGenerator.php +++ b/client/includes/Hooks/LinkedDataSchemaGenerator.php @@ -4,7 +4,6 @@ use File; use MediaWiki\Html\Html; -use MediaWiki\Language\Language; use MediaWiki\Output\Hook\OutputPageParserOutputHook; use MediaWiki\Output\OutputPage; use MediaWiki\Parser\ParserOutput; @@ -13,28 +12,18 @@ use PageImages\PageImages; use Wikibase\Client\RepoLinker; use Wikibase\DataModel\Entity\EntityId; -use Wikibase\DataModel\Services\Lookup\TermLookup; -use Wikibase\DataModel\Services\Lookup\TermLookupException; /** * @license GPL-2.0-or-later */ class LinkedDataSchemaGenerator implements OutputPageParserOutputHook { - /** @var string */ - private $langCode; /** @var RepoLinker */ private $repoLinker; - /** @var TermLookup */ - private $termLookup; public function __construct( - Language $language, - RepoLinker $repoLinker, - TermLookup $termLookup + RepoLinker $repoLinker ) { - $this->langCode = $language->getCode(); $this->repoLinker = $repoLinker; - $this->termLookup = $termLookup; } /** @@ -42,6 +31,7 @@ public function __construct( * @param string|null $revisionTimestamp * @param string|null $firstRevisionTimestamp * @param EntityId $entityId + * @param string|null $description * * @return string */ @@ -49,11 +39,11 @@ public function createSchemaElement( Title $title, ?string $revisionTimestamp, ?string $firstRevisionTimestamp, - EntityId $entityId + EntityId $entityId, + ?string $description ): string { $entityConceptUri = $this->repoLinker->getEntityConceptUri( $entityId ); $imageFile = $this->queryPageImage( $title ); - $description = $this->getDescription( $entityId ); $schema = $this->createSchema( $title, $revisionTimestamp, $firstRevisionTimestamp, $entityConceptUri, $imageFile, $description ); @@ -64,7 +54,7 @@ public function createSchemaElement( return $html; } - public function createSchema( + private function createSchema( Title $title, ?string $revisionTimestamp, ?string $firstRevisionTimestamp, @@ -127,16 +117,6 @@ private function queryPageImage( Title $title ) { return PageImages::getPageImage( $title ) ?: null; } - private function getDescription( EntityId $entityId ): string { - try { - $description = $this->termLookup->getDescription( $entityId, $this->langCode ); - } catch ( TermLookupException $exception ) { - return ''; - } - - return $description ?: ''; - } - /** * Add output page properties to be consumed in the LinkedDataSchemaGenerator * @@ -148,5 +128,10 @@ public function onOutputPageParserOutput( $outputPage, $parserOutput ): void { if ( $firstRevisionTimestamp !== null ) { $outputPage->setProperty( 'first_revision_timestamp', $firstRevisionTimestamp ); } + + $description = $parserOutput->getExtensionData( 'wikibase_item_description' ); + if ( $description !== null ) { + $outputPage->setProperty( 'wikibase_item_description', $description ); + } } } diff --git a/client/includes/Hooks/ParserOutputUpdateHookHandler.php b/client/includes/Hooks/ParserOutputUpdateHookHandler.php index 1d7533bbc84..17dd479049e 100644 --- a/client/includes/Hooks/ParserOutputUpdateHookHandler.php +++ b/client/includes/Hooks/ParserOutputUpdateHookHandler.php @@ -81,6 +81,7 @@ public function doContentAlterParserOutput( Content $content, Title $title, Pars $usageAccumulator = $this->usageAccumulatorFactory->newFromParserOutputProvider( $parserOutputProvider ); $langLinkHandler = $this->langLinkHandlerFactory->getLangLinkHandler( $usageAccumulator ); $useRepoLinks = $langLinkHandler->useRepoLinks( $title, $parserOutput ); + $langCode = $content->getContentHandler()->getPageLanguage( $title )->getCode(); if ( $useRepoLinks ) { // add links @@ -93,6 +94,7 @@ public function doContentAlterParserOutput( Content $content, Title $title, Pars $this->parserOutputDataUpdater->updateUnconnectedPageProperty( $content, $title, $parserOutputProvider ); $this->parserOutputDataUpdater->updateBadgesProperty( $title, $parserOutputProvider ); $this->parserOutputDataUpdater->updateFirstRevisionTimestampProperty( $title, $parserOutputProvider ); + $this->parserOutputDataUpdater->updateWikibaseItemDescriptionProperty( $title, $parserOutputProvider, $langCode ); $parserOutputProvider->close(); } diff --git a/client/includes/ParserOutput/ClientParserOutputDataUpdater.php b/client/includes/ParserOutput/ClientParserOutputDataUpdater.php index 1793aecb6bb..0b9c5e0a552 100644 --- a/client/includes/ParserOutput/ClientParserOutputDataUpdater.php +++ b/client/includes/ParserOutput/ClientParserOutputDataUpdater.php @@ -16,6 +16,8 @@ use Wikibase\DataModel\Entity\Item; use Wikibase\DataModel\Entity\ItemId; use Wikibase\DataModel\Services\Lookup\EntityLookup; +use Wikibase\DataModel\Services\Lookup\TermLookup; +use Wikibase\DataModel\Services\Lookup\TermLookupException; use Wikibase\Lib\Store\SiteLinkLookup; /** @@ -60,6 +62,9 @@ class ClientParserOutputDataUpdater { /** @var RevisionLookup */ private $revisionLookup; + /** @var TermLookup */ + private $termLookup; + /** * @param OtherProjectsSidebarGeneratorFactory $otherProjectsSidebarGeneratorFactory * Use the factory here to defer initialization of things like Site objects. @@ -68,6 +73,7 @@ class ClientParserOutputDataUpdater { * @param UsageAccumulatorFactory $usageAccumulatorFactory * @param string $siteId The global site ID for the local wiki * @param RevisionLookup $revisionLookup + * @param TermLookup $termLookup * @param LoggerInterface|null $logger * * @throws InvalidArgumentException @@ -79,6 +85,7 @@ public function __construct( UsageAccumulatorFactory $usageAccumulatorFactory, string $siteId, RevisionLookup $revisionLookup, + TermLookup $termLookup, ?LoggerInterface $logger = null ) { $this->otherProjectsSidebarGeneratorFactory = $otherProjectsSidebarGeneratorFactory; @@ -88,6 +95,7 @@ public function __construct( $this->siteId = $siteId; $this->logger = $logger ?: new NullLogger(); $this->revisionLookup = $revisionLookup; + $this->termLookup = $termLookup; } /** @@ -217,6 +225,28 @@ public function updateFirstRevisionTimestampProperty( $title, $parserOutputProvi ); } + public function updateWikibaseItemDescriptionProperty( + Title $title, + ParserOutputProvider $parserOutputProvider, + string $langCode + ): void { + $itemId = $this->getItemIdForTitle( $title ); + if ( $itemId ) { + try { + /** + * We do not need to add description usage tracking here, + * because it is already tracked by {@link ImplicitDescriptionUsageLookup} + */ + $description = $this->termLookup->getDescription( $itemId, $langCode ); + } catch ( TermLookupException $exception ) { + $description = ''; + } + $parserOutputProvider->getParserOutput()->setExtensionData( + 'wikibase_item_description', $description + ); + } + } + private function getItemIdForTitle( Title $title ): ?ItemId { return $this->siteLinkLookup->getItemIdForLink( $this->siteId, diff --git a/client/includes/Usage/ImplicitDescriptionUsageLookup.php b/client/includes/Usage/ImplicitDescriptionUsageLookup.php index 3fe67588391..d1590cd4f4b 100644 --- a/client/includes/Usage/ImplicitDescriptionUsageLookup.php +++ b/client/includes/Usage/ImplicitDescriptionUsageLookup.php @@ -31,7 +31,9 @@ * unless the client page also has a local description overriding the central one. * This is because the description is used, for example, * as part of the search result for the page (typically on mobile), - * even if it is never used in the page itself. + * even if it is never used in the page itself. The item's description is also + * referenced in the page's LinkedDataSchema, so implicit usage tracking ensures + * the cached schema remains up-to-date. * * @see @ref docs_topics_usagetracking for virtual usage, * a similar but separate concept. diff --git a/client/tests/phpunit/integration/includes/ClientParserOutputDataUpdaterTest.php b/client/tests/phpunit/integration/includes/ClientParserOutputDataUpdaterTest.php index 12ea9635e8a..12c83b4bc31 100644 --- a/client/tests/phpunit/integration/includes/ClientParserOutputDataUpdaterTest.php +++ b/client/tests/phpunit/integration/includes/ClientParserOutputDataUpdaterTest.php @@ -23,6 +23,7 @@ use Wikibase\DataModel\Entity\Item; use Wikibase\DataModel\Entity\ItemId; use Wikibase\DataModel\Services\Lookup\EntityRedirectTargetLookup; +use Wikibase\DataModel\Services\Lookup\TermLookup; use Wikibase\DataModel\SiteLinkList; use Wikibase\Lib\Tests\MockRepository; @@ -88,7 +89,8 @@ private function newInstance( $this->mockRepo, $this->newUsageAccumulatorFactory(), 'srwiki', - $this->createMock( RevisionLookup::class ) + $this->createMock( RevisionLookup::class ), + $this->createMock( TermLookup::class ) ); } @@ -287,6 +289,7 @@ public function testUpdateBadgesProperty_inconsistentSiteLinkLookupEmptySiteLink $this->newUsageAccumulatorFactory(), 'srwiki', $this->createMock( RevisionLookup::class ), + $this->createMock( TermLookup::class ), $logger ); @@ -318,6 +321,7 @@ public function testUpdateBadgesProperty_inconsistentSiteLinkLookupNoSuchEntity( $this->newUsageAccumulatorFactory(), 'srwiki', $this->createMock( RevisionLookup::class ), + $this->createMock( TermLookup::class ), $logger ); diff --git a/client/tests/phpunit/integration/includes/Hooks/ParserOutputUpdateHookHandlerTest.php b/client/tests/phpunit/integration/includes/Hooks/ParserOutputUpdateHookHandlerTest.php index 358c070ba85..581a1beabf4 100644 --- a/client/tests/phpunit/integration/includes/Hooks/ParserOutputUpdateHookHandlerTest.php +++ b/client/tests/phpunit/integration/includes/Hooks/ParserOutputUpdateHookHandlerTest.php @@ -5,6 +5,7 @@ namespace Wikibase\Client\Tests\Integration\Hooks; use MediaWiki\Content\Content; +use MediaWiki\Content\ContentHandler; use MediaWiki\HookContainer\HookContainer; use MediaWiki\Parser\ParserOutput; use MediaWiki\Revision\RevisionLookup; @@ -35,6 +36,7 @@ use Wikibase\DataModel\Services\Lookup\EntityRedirectTargetLookup; use Wikibase\DataModel\Services\Lookup\InMemoryEntityLookup; use Wikibase\DataModel\Services\Lookup\LabelDescriptionLookup; +use Wikibase\DataModel\Services\Lookup\TermLookup; use Wikibase\DataModel\SiteLink; use Wikibase\DataModel\SiteLinkList; use Wikibase\DataModel\Snak\PropertyValueSnak; @@ -324,7 +326,7 @@ public function testDoContentAlterParserOutput( ) { $titleWrapper = TestingAccessWrapper::newFromObject( $title ); $titleWrapper->mRedirect = false; - $content = $this->createMock( Content::class ); + $content = $this->mockContent(); $parserOutput = $this->newParserOutput( $extensionDataAppend, [] ); $handler = $this->newParserOutputUpdateHookHandler( $this->getTestSiteLinkData() ); @@ -354,7 +356,7 @@ public function testDoContentAlterParserOutput( } public function testDoContentAlterParserOutput_sitelinkOfNoItem() { - $content = $this->createMock( Content::class ); + $content = $this->mockContent(); $title = Title::makeTitle( NS_MAIN, 'Plutonium' ); $parserOutput = $this->newParserOutput( [], [] ); @@ -422,7 +424,7 @@ public function testGivenSitelinkHasStatementWithUnknownEntityType_linkDataIsAdd $this->newUsageAccumulatorFactory() ); - $content = $this->createMock( Content::class ); + $content = $this->mockContent(); $title = Title::makeTitle( NS_MAIN, 'Foobarium' ); $titleWrapper = TestingAccessWrapper::newFromObject( $title ); $titleWrapper->mRedirect = false; @@ -480,7 +482,8 @@ private function newParserOutputDataUpdater( MockRepository $mockRepo, array $si $mockRepo, $this->newUsageAccumulatorFactory(), $settings->getSetting( 'siteGlobalID' ), - $this->createMock( RevisionLookup::class ) + $this->createMock( RevisionLookup::class ), + $this->createMock( TermLookup::class ) ); } @@ -500,4 +503,16 @@ private function newLangLinkHandlerFactory( $namespaceChecker, $mockRepo ) { ); } + private function mockContent() { + $contentHandler = $this->createMock( ContentHandler::class ); + $contentHandler->method( 'getPageLanguage' ) + ->willReturn( $this->getServiceContainer()->getLanguageFactory()->getLanguage( 'en' ) ); + + $content = $this->createMock( Content::class ); + $content->method( 'getContentHandler' ) + ->willReturn( $contentHandler ); + + return $content; + } + } diff --git a/client/tests/phpunit/integration/includes/Hooks/SkinAfterBottomScriptsHandlerTest.php b/client/tests/phpunit/integration/includes/Hooks/SkinAfterBottomScriptsHandlerTest.php index b08e08de7ea..74a105065ce 100644 --- a/client/tests/phpunit/integration/includes/Hooks/SkinAfterBottomScriptsHandlerTest.php +++ b/client/tests/phpunit/integration/includes/Hooks/SkinAfterBottomScriptsHandlerTest.php @@ -3,13 +3,12 @@ namespace Wikibase\Client\Tests\Unit\Hooks; use File; -use MediaWiki\Language\Language; use MediaWiki\Title\Title; use Wikibase\Client\Hooks\LinkedDataSchemaGenerator; use Wikibase\Client\RepoLinker; -use Wikibase\Client\WikibaseClient; use Wikibase\DataAccess\EntitySourceDefinitions; use Wikibase\Lib\SubEntityTypesMapper; +use Wikimedia\TestingAccessWrapper; /** * @covers \Wikibase\Client\Hooks\LinkedDataSchemaGenerator @@ -34,15 +33,14 @@ public function testCreateSchema( $revisionTimestamp, callable $imageFactory, $d '/w' ); $generator = new LinkedDataSchemaGenerator( - $this->createMock( Language::class ), $repoLinker, - WikibaseClient::getTermLookup() ); + $generatorWrapper = TestingAccessWrapper::newFromObject( $generator ); $title = $this->mockTitle( 'https://de.wikipedia.org/wiki', 'Douglas Adams' ); $firstRevisionTimestamp = "2002-05-27T18:26:23Z"; - $actual = $generator->createSchema( + $actual = $generatorWrapper->createSchema( $title, $revisionTimestamp, $firstRevisionTimestamp, 'https://www.wikidata.org/entity/Q42', $image, $description ); $this->assertSchemaSubset( $expected, $actual ); @@ -136,5 +134,4 @@ private function mockTitle( $baseURL, $titleText ) { ->willReturn( $titleText ); return $mock; } - } diff --git a/client/tests/phpunit/unit/includes/ServiceWiring/ParserOutputDataUpdaterTest.php b/client/tests/phpunit/unit/includes/ServiceWiring/ParserOutputDataUpdaterTest.php index 35be81b6886..44811264214 100644 --- a/client/tests/phpunit/unit/includes/ServiceWiring/ParserOutputDataUpdaterTest.php +++ b/client/tests/phpunit/unit/includes/ServiceWiring/ParserOutputDataUpdaterTest.php @@ -11,6 +11,7 @@ use Wikibase\Client\Tests\Unit\ServiceWiringTestCase; use Wikibase\Client\Usage\UsageAccumulatorFactory; use Wikibase\DataModel\Services\Lookup\InMemoryEntityLookup; +use Wikibase\DataModel\Services\Lookup\TermLookup; use Wikibase\Lib\SettingsArray; use Wikibase\Lib\Store\HashSiteLinkStore; @@ -42,6 +43,7 @@ public function testConstruction(): void { new NullLogger() ); $this->mockService( 'WikibaseClient.UsageAccumulatorFactory', $this->createMock( UsageAccumulatorFactory::class ) ); + $this->mockService( 'WikibaseClient.TermLookup', $this->createMock( TermLookup::class ) ); $this->assertInstanceOf( ClientParserOutputDataUpdater::class, $this->getService( 'WikibaseClient.ParserOutputDataUpdater' ) diff --git a/docs/topics/usagetracking.md b/docs/topics/usagetracking.md index 75b5a3596f6..3cd0ef44a02 100644 --- a/docs/topics/usagetracking.md +++ b/docs/topics/usagetracking.md @@ -89,6 +89,7 @@ based on the old and new title in the sitelink, so that both get updated. not directly related to any change to the entity. [ImplicitDescriptionUsageLookup] adds implicit usages on the descriptions of items linked to local pages, so that description edits are added to the recent changes even if the descriptions are not used directly. +This also keeps SEO features like the cached schema built in [LinkedDataSchemaGenerator] up-to-date. ### Repo side usage tracking diff --git a/extension-client.json b/extension-client.json index c3fed35300a..2d37560f7ff 100644 --- a/extension-client.json +++ b/extension-client.json @@ -171,9 +171,7 @@ "LinkedDataSchemaGenerator": { "class": "\\Wikibase\\Client\\Hooks\\LinkedDataSchemaGenerator", "services": [ - "ContentLanguage", - "WikibaseClient.RepoLinker", - "WikibaseClient.TermLookup" + "WikibaseClient.RepoLinker" ] }, "LoadExtensionSchemaUpdates": {