Skip to content

Commit

Permalink
Merge "Revert "Cache wb item description for linked data schema""
Browse files Browse the repository at this point in the history
  • Loading branch information
jenkins-bot authored and Gerrit Code Review committed Jan 27, 2025
2 parents 7538a44 + 82c8ab0 commit a0b86dc
Show file tree
Hide file tree
Showing 12 changed files with 51 additions and 87 deletions.
3 changes: 1 addition & 2 deletions client/WikibaseClient.ServiceWiring.php
Original file line number Diff line number Diff line change
Expand Up @@ -655,8 +655,7 @@ function ( EntityNamespaceLookup $nsLookup, DatabaseEntitySource $source ): Enti
WikibaseClient::getUsageAccumulatorFactory( $services ),
$settings->getSetting( 'siteGlobalID' ),
$services->getRevisionLookup(),
WikibaseClient::getTermLookup( $services ),
WikibaseClient::getLogger( $services ),
WikibaseClient::getLogger( $services )
);
},

Expand Down
19 changes: 10 additions & 9 deletions client/includes/ClientHooks.php
Original file line number Diff line number Diff line change
Expand Up @@ -182,18 +182,20 @@ public static function onCirrusSearchAddQueryFeatures(
*
* @return bool Always true.
*/
public static function onSkinAfterBottomScripts( Skin $skin, string &$html ) {
public static function onSkinAfterBottomScripts( Skin $skin, &$html ) {
$services = MediaWikiServices::getInstance();
$enabledNamespaces = WikibaseClient::getSettings( $services )
->getSetting( 'pageSchemaNamespaces' );

$generator = new LinkedDataSchemaGenerator(
$services->getContentLanguage(),
WikibaseClient::getRepoLinker( $services ),
WikibaseClient::getTermLookup( $services ),
);

$outputPage = $skin->getOutput();
$entityId = self::parseEntityId( $outputPage->getProperty( 'wikibase_item' ) );
$title = $outputPage->getTitle();
$out = $skin->getOutput();
$entityId = self::parseEntityId( $out->getProperty( 'wikibase_item' ) );
$title = $out->getTitle();
if (
!$entityId ||
!$title ||
Expand All @@ -203,15 +205,14 @@ public static function onSkinAfterBottomScripts( Skin $skin, string &$html ) {
return true;
}

$revisionTimestamp = $outputPage->getRevisionTimestamp();
$firstRevisionTimestamp = $outputPage->getProperty( 'first_revision_timestamp' );
$description = $outputPage->getProperty( 'wikibase_item_description' );
$revisionTimestamp = $out->getRevisionTimestamp();
$firstRevisionTimestamp = $out->getProperty( 'first_revision_timestamp' );

$html .= $generator->createSchemaElement(
$title,
$revisionTimestamp,
$firstRevisionTimestamp,
$entityId,
$description
$entityId
);

return true;
Expand Down
35 changes: 25 additions & 10 deletions client/includes/Hooks/LinkedDataSchemaGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use File;
use MediaWiki\Html\Html;
use MediaWiki\Language\Language;
use MediaWiki\Output\Hook\OutputPageParserOutputHook;
use MediaWiki\Output\OutputPage;
use MediaWiki\Parser\ParserOutput;
Expand All @@ -12,38 +13,47 @@
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(
RepoLinker $repoLinker
Language $language,
RepoLinker $repoLinker,
TermLookup $termLookup
) {
$this->langCode = $language->getCode();
$this->repoLinker = $repoLinker;
$this->termLookup = $termLookup;
}

/**
* @param Title $title
* @param string|null $revisionTimestamp
* @param string|null $firstRevisionTimestamp
* @param EntityId $entityId
* @param string $description
*
* @return string
*/
public function createSchemaElement(
Title $title,
?string $revisionTimestamp,
?string $firstRevisionTimestamp,
EntityId $entityId,
string $description
EntityId $entityId
): string {
$entityConceptUri = $this->repoLinker->getEntityConceptUri( $entityId );
$imageFile = $this->queryPageImage( $title );
$description = $this->getDescription( $entityId );
$schema = $this->createSchema(
$title, $revisionTimestamp, $firstRevisionTimestamp, $entityConceptUri, $imageFile, $description
);
Expand All @@ -54,7 +64,7 @@ public function createSchemaElement(
return $html;
}

private function createSchema(
public function createSchema(
Title $title,
?string $revisionTimestamp,
?string $firstRevisionTimestamp,
Expand Down Expand Up @@ -117,6 +127,16 @@ 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
*
Expand All @@ -128,10 +148,5 @@ 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 );
}
}
}
2 changes: 0 additions & 2 deletions client/includes/Hooks/ParserOutputUpdateHookHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ 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
Expand All @@ -94,7 +93,6 @@ 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();
}

Expand Down
30 changes: 0 additions & 30 deletions client/includes/ParserOutput/ClientParserOutputDataUpdater.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
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;

/**
Expand Down Expand Up @@ -62,9 +60,6 @@ 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.
Expand All @@ -73,7 +68,6 @@ 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
Expand All @@ -85,7 +79,6 @@ public function __construct(
UsageAccumulatorFactory $usageAccumulatorFactory,
string $siteId,
RevisionLookup $revisionLookup,
TermLookup $termLookup,
?LoggerInterface $logger = null
) {
$this->otherProjectsSidebarGeneratorFactory = $otherProjectsSidebarGeneratorFactory;
Expand All @@ -95,7 +88,6 @@ public function __construct(
$this->siteId = $siteId;
$this->logger = $logger ?: new NullLogger();
$this->revisionLookup = $revisionLookup;
$this->termLookup = $termLookup;
}

/**
Expand Down Expand Up @@ -225,28 +217,6 @@ 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,
Expand Down
4 changes: 1 addition & 3 deletions client/includes/Usage/ImplicitDescriptionUsageLookup.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@
* 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. The item's description is also
* referenced in the page's LinkedDataSchema, so implicit usage tracking ensures
* the cached schema remains up-to-date.
* even if it is never used in the page itself.
*
* @see @ref docs_topics_usagetracking for virtual usage,
* a similar but separate concept.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
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;

Expand Down Expand Up @@ -89,8 +88,7 @@ private function newInstance(
$this->mockRepo,
$this->newUsageAccumulatorFactory(),
'srwiki',
$this->createMock( RevisionLookup::class ),
$this->createMock( TermLookup::class )
$this->createMock( RevisionLookup::class )
);
}

Expand Down Expand Up @@ -289,7 +287,6 @@ public function testUpdateBadgesProperty_inconsistentSiteLinkLookupEmptySiteLink
$this->newUsageAccumulatorFactory(),
'srwiki',
$this->createMock( RevisionLookup::class ),
$this->createMock( TermLookup::class ),
$logger
);

Expand Down Expand Up @@ -321,7 +318,6 @@ public function testUpdateBadgesProperty_inconsistentSiteLinkLookupNoSuchEntity(
$this->newUsageAccumulatorFactory(),
'srwiki',
$this->createMock( RevisionLookup::class ),
$this->createMock( TermLookup::class ),
$logger
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
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;
Expand Down Expand Up @@ -36,7 +35,6 @@
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;
Expand Down Expand Up @@ -326,7 +324,7 @@ public function testDoContentAlterParserOutput(
) {
$titleWrapper = TestingAccessWrapper::newFromObject( $title );
$titleWrapper->mRedirect = false;
$content = $this->mockContent();
$content = $this->createMock( Content::class );
$parserOutput = $this->newParserOutput( $extensionDataAppend, [] );
$handler = $this->newParserOutputUpdateHookHandler( $this->getTestSiteLinkData() );

Expand Down Expand Up @@ -356,7 +354,7 @@ public function testDoContentAlterParserOutput(
}

public function testDoContentAlterParserOutput_sitelinkOfNoItem() {
$content = $this->mockContent();
$content = $this->createMock( Content::class );
$title = Title::makeTitle( NS_MAIN, 'Plutonium' );

$parserOutput = $this->newParserOutput( [], [] );
Expand Down Expand Up @@ -424,7 +422,7 @@ public function testGivenSitelinkHasStatementWithUnknownEntityType_linkDataIsAdd
$this->newUsageAccumulatorFactory()
);

$content = $this->mockContent();
$content = $this->createMock( Content::class );
$title = Title::makeTitle( NS_MAIN, 'Foobarium' );
$titleWrapper = TestingAccessWrapper::newFromObject( $title );
$titleWrapper->mRedirect = false;
Expand Down Expand Up @@ -482,8 +480,7 @@ private function newParserOutputDataUpdater( MockRepository $mockRepo, array $si
$mockRepo,
$this->newUsageAccumulatorFactory(),
$settings->getSetting( 'siteGlobalID' ),
$this->createMock( RevisionLookup::class ),
$this->createMock( TermLookup::class )
$this->createMock( RevisionLookup::class )
);
}

Expand All @@ -503,16 +500,4 @@ 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;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
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
Expand All @@ -33,14 +34,15 @@ 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 = $generatorWrapper->createSchema(
$actual = $generator->createSchema(
$title, $revisionTimestamp, $firstRevisionTimestamp, 'https://www.wikidata.org/entity/Q42', $image, $description
);
$this->assertSchemaSubset( $expected, $actual );
Expand Down Expand Up @@ -134,4 +136,5 @@ private function mockTitle( $baseURL, $titleText ) {
->willReturn( $titleText );
return $mock;
}

}
Loading

0 comments on commit a0b86dc

Please sign in to comment.