Skip to content

Commit

Permalink
Merge "Fix wikidata entity usage tracking and access count"
Browse files Browse the repository at this point in the history
  • Loading branch information
jenkins-bot authored and Gerrit Code Review committed Nov 15, 2024
2 parents befbb01 + 6c42696 commit c54814b
Show file tree
Hide file tree
Showing 17 changed files with 155 additions and 89 deletions.
10 changes: 5 additions & 5 deletions client/WikibaseClient.ServiceWiring.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
use Wikibase\DataModel\Services\Lookup\EntityLookup;
use Wikibase\DataModel\Services\Lookup\EntityRetrievingDataTypeLookup;
use Wikibase\DataModel\Services\Lookup\PropertyDataTypeLookup;
use Wikibase\DataModel\Services\Lookup\RestrictedEntityLookup;
use Wikibase\DataModel\Services\Lookup\RestrictedEntityLookupFactory;
use Wikibase\DataModel\Services\Lookup\TermLookup;
use Wikibase\DataModel\Services\Term\PropertyLabelResolver;
use Wikibase\DataModel\Services\Term\TermBuffer;
Expand Down Expand Up @@ -781,7 +781,7 @@ function ( EntityNamespaceLookup $nsLookup, DatabaseEntitySource $source ): Enti
WikibaseClient::getStatementGroupRendererFactory( $services ),
WikibaseClient::getStore( $services )->getSiteLinkLookup(),
WikibaseClient::getEntityIdParser( $services ),
WikibaseClient::getRestrictedEntityLookup( $services ),
WikibaseClient::getRestrictedEntityLookupFactory( $services ),
$settings->getSetting( 'siteGlobalID' ),
$settings->getSetting( 'allowArbitraryDataAccess' )
);
Expand Down Expand Up @@ -868,13 +868,13 @@ function ( EntityNamespaceLookup $nsLookup, DatabaseEntitySource $source ): Enti
);
},

'WikibaseClient.RestrictedEntityLookup' => function ( MediaWikiServices $services ): RestrictedEntityLookup {
'WikibaseClient.RestrictedEntityLookupFactory' => function ( MediaWikiServices $services ): RestrictedEntityLookupFactory {
$settings = WikibaseClient::getSettings( $services );
$disabledEntityTypesEntityLookup = new DisabledEntityTypesEntityLookup(
WikibaseClient::getEntityLookup( $services ),
$settings->getSetting( 'disabledAccessEntityTypes' )
);
return new RestrictedEntityLookup(
return new RestrictedEntityLookupFactory(
$disabledEntityTypesEntityLookup,
$settings->getSetting( 'entityAccessLimit' )
);
Expand Down Expand Up @@ -969,7 +969,7 @@ function ( EntityNamespaceLookup $nsLookup, DatabaseEntitySource $source ): Enti
return new StatementGroupRendererFactory(
WikibaseClient::getPropertyLabelResolver( $services ),
new SnaksFinder(),
WikibaseClient::getRestrictedEntityLookup( $services ),
WikibaseClient::getRestrictedEntityLookupFactory( $services ),
WikibaseClient::getDataAccessSnakFormatterFactory( $services ),
WikibaseClient::getUsageAccumulatorFactory( $services ),
$services->getLanguageConverterFactory(),
Expand Down
14 changes: 7 additions & 7 deletions client/includes/DataAccess/ParserFunctions/Runner.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\EntityIdParser;
use Wikibase\DataModel\Entity\EntityIdParsingException;
use Wikibase\DataModel\Services\Lookup\RestrictedEntityLookup;
use Wikibase\DataModel\Services\Lookup\RestrictedEntityLookupFactory;
use Wikibase\Lib\Store\SiteLinkLookup;

/**
Expand All @@ -36,9 +36,9 @@ class Runner {
private $entityIdParser;

/**
* @var RestrictedEntityLookup
* @var RestrictedEntityLookupFactory
*/
private $restrictedEntityLookup;
private $restrictedEntityLookupFactory;

/**
* @var string
Expand All @@ -54,22 +54,22 @@ class Runner {
* @param StatementGroupRendererFactory $rendererFactory
* @param SiteLinkLookup $siteLinkLookup
* @param EntityIdParser $entityIdParser
* @param RestrictedEntityLookup $restrictedEntityLookup
* @param RestrictedEntityLookupFactory $restrictedEntityLookupFactory
* @param string $siteId
* @param bool $allowArbitraryDataAccess
*/
public function __construct(
StatementGroupRendererFactory $rendererFactory,
SiteLinkLookup $siteLinkLookup,
EntityIdParser $entityIdParser,
RestrictedEntityLookup $restrictedEntityLookup,
RestrictedEntityLookupFactory $restrictedEntityLookupFactory,
$siteId,
$allowArbitraryDataAccess
) {
$this->rendererFactory = $rendererFactory;
$this->siteLinkLookup = $siteLinkLookup;
$this->entityIdParser = $entityIdParser;
$this->restrictedEntityLookup = $restrictedEntityLookup;
$this->restrictedEntityLookupFactory = $restrictedEntityLookupFactory;
$this->siteId = $siteId;
$this->allowArbitraryDataAccess = $allowArbitraryDataAccess;
}
Expand Down Expand Up @@ -141,7 +141,7 @@ private function getEntityIdFromString( Parser $parser, $entityIdString ) {

// Getting a foreign item is expensive (unless we already loaded it and it's cached)
if (
!$this->restrictedEntityLookup->entityHasBeenAccessed( $entityId ) &&
!$this->restrictedEntityLookupFactory->getRestrictedEntityLookup( $parser )->entityHasBeenAccessed( $entityId ) &&
!$parser->incrementExpensiveFunctionCount()
) {
// Just do nothing, that's what parser functions do when the limit has been
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
use Wikibase\Client\DataAccess\StatementTransclusionInteractor;
use Wikibase\Client\Usage\UsageAccumulator;
use Wikibase\Client\Usage\UsageAccumulatorFactory;
use Wikibase\DataModel\Services\Lookup\EntityLookup;
use Wikibase\DataModel\Services\Lookup\RestrictedEntityLookupFactory;
use Wikibase\DataModel\Services\Term\PropertyLabelResolver;

/**
Expand All @@ -42,9 +42,9 @@ class StatementGroupRendererFactory {
private $languageAwareRenderers = [];

/**
* @var EntityLookup
* @var RestrictedEntityLookupFactory
*/
private $entityLookup;
private $restrictedEntityLookupFactory;

/**
* @var DataAccessSnakFormatterFactory
Expand Down Expand Up @@ -74,7 +74,7 @@ class StatementGroupRendererFactory {
public function __construct(
PropertyLabelResolver $propertyLabelResolver,
SnaksFinder $snaksFinder,
EntityLookup $entityLookup,
RestrictedEntityLookupFactory $restrictedEntityLookupFactory,
DataAccessSnakFormatterFactory $dataAccessSnakFormatterFactory,
UsageAccumulatorFactory $usageAccumulatorFactory,
LanguageConverterFactory $langConvFactory,
Expand All @@ -83,7 +83,7 @@ public function __construct(
) {
$this->propertyLabelResolver = $propertyLabelResolver;
$this->snaksFinder = $snaksFinder;
$this->entityLookup = $entityLookup;
$this->restrictedEntityLookupFactory = $restrictedEntityLookupFactory;
$this->dataAccessSnakFormatterFactory = $dataAccessSnakFormatterFactory;
$this->usageAccumulatorFactory = $usageAccumulatorFactory;
$this->langConvFactory = $langConvFactory;
Expand All @@ -109,6 +109,7 @@ public function newRendererFromParser(
$type,
$targetLanguage,
$usageAccumulator,
$parser,
$parser->getOutput(),
$parser->getTitle()
);
Expand All @@ -118,6 +119,7 @@ public function newRendererFromParser(
$type,
$variants,
$usageAccumulator,
$parser,
$parser->getOutput(),
$parser->getTitle()
);
Expand All @@ -127,6 +129,7 @@ public function newRendererFromParser(
$type,
$targetLanguage,
$usageAccumulator,
$parser,
$parser->getOutput(),
$parser->getTitle()
);
Expand All @@ -137,13 +140,16 @@ public function newRendererFromParser(
* @param string $type One of DataAccessSnakFormatterFactory::TYPE_*
* @param Language $language
* @param UsageAccumulator $usageAccumulator
* @param Parser $parser
* @param ParserOutput $parserOutput
* @param Title $title
* @return LanguageAwareRenderer
*/
private function newLanguageAwareRenderer(
string $type,
Language $language,
UsageAccumulator $usageAccumulator,
Parser $parser,
ParserOutput $parserOutput,
Title $title
): LanguageAwareRenderer {
Expand All @@ -154,7 +160,7 @@ private function newLanguageAwareRenderer(
);

$propertyIdResolver = new PropertyIdResolver(
$this->entityLookup,
$this->restrictedEntityLookupFactory->getRestrictedEntityLookup( $parser ),
$this->propertyLabelResolver,
$usageAccumulator
);
Expand All @@ -164,7 +170,7 @@ private function newLanguageAwareRenderer(
$propertyIdResolver,
$this->snaksFinder,
$snakFormatter,
$this->entityLookup,
$this->restrictedEntityLookupFactory->getRestrictedEntityLookup( $parser ),
$usageAccumulator
);

Expand All @@ -180,13 +186,16 @@ private function newLanguageAwareRenderer(
* @param string $type One of DataAccessSnakFormatterFactory::TYPE_*
* @param string $languageCode
* @param UsageAccumulator $usageAccumulator
* @param Parser $parser
* @param ParserOutput $parserOutput
* @param Title $title
* @return LanguageAwareRenderer
*/
private function getLanguageAwareRendererFromCode(
string $type,
string $languageCode,
UsageAccumulator $usageAccumulator,
Parser $parser,
ParserOutput $parserOutput,
Title $title
): LanguageAwareRenderer {
Expand All @@ -195,6 +204,7 @@ private function getLanguageAwareRendererFromCode(
$type,
$this->langFactory->getLanguage( $languageCode ),
$usageAccumulator,
$parser,
$parserOutput,
$title
);
Expand All @@ -207,6 +217,7 @@ private function getLanguageAwareRendererFromCode(
* @param string $type One of DataAccessSnakFormatterFactory::TYPE_*
* @param string[] $variants
* @param UsageAccumulator $usageAccumulator
* @param Parser $parser
* @param ParserOutput $parserOutput
* @param Title $title
*
Expand All @@ -216,6 +227,7 @@ private function newVariantsAwareRenderer(
string $type,
array $variants,
UsageAccumulator $usageAccumulator,
Parser $parser,
ParserOutput $parserOutput,
Title $title
): VariantsAwareRenderer {
Expand All @@ -226,6 +238,7 @@ private function newVariantsAwareRenderer(
$type,
$variant,
$usageAccumulator,
$parser,
$parserOutput,
$title
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ private function newImplementation(): WikibaseLuaEntityBindings {
$lang
);

$entityLookup = WikibaseClient::getRestrictedEntityLookup();
$parser = $this->getParser();
$entityLookup = WikibaseClient::getRestrictedEntityLookupFactory()->getRestrictedEntityLookup( $parser );

$propertyIdResolver = new PropertyIdResolver(
$entityLookup,
Expand Down
4 changes: 3 additions & 1 deletion client/includes/DataAccess/Scribunto/WikibaseLibrary.php
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,11 @@ private function allowDataAccessInUserLanguage(): bool {
}

private function newEntityAccessor(): EntityAccessor {
$parser = $this->getParser();

return new EntityAccessor(
$this->getEntityIdParser(),
WikibaseClient::getRestrictedEntityLookup(),
WikibaseClient::getRestrictedEntityLookupFactory()->getRestrictedEntityLookup( $parser ),
$this->getUsageAccumulator(),
WikibaseClient::getCompactEntitySerializer(),
WikibaseClient::getCompactBaseDataModelSerializerFactory()
Expand Down
21 changes: 9 additions & 12 deletions client/includes/Hooks/ParserHookHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use MediaWiki\Hook\ParserLimitReportPrepareHook;
use MediaWiki\Parser\Parser;
use MediaWiki\Parser\ParserOutput;
use Wikibase\DataModel\Services\Lookup\RestrictedEntityLookup;
use Wikibase\DataModel\Services\Lookup\RestrictedEntityLookupFactory;
use Wikibase\Lib\SettingsArray;

/**
Expand All @@ -21,27 +21,24 @@ class ParserHookHandler implements
ParserClearStateHook,
ParserLimitReportPrepareHook
{
private RestrictedEntityLookupFactory $restrictedEntityLookupFactory;

/** @var RestrictedEntityLookup */
private $restrictedEntityLookup;

/** @var int */
private $entityAccessLimit;
private int $entityAccessLimit;

public function __construct(
RestrictedEntityLookup $restrictedEntityLookup,
RestrictedEntityLookupFactory $restrictedEntityLookupFactory,
int $entityAccessLimit
) {
$this->restrictedEntityLookup = $restrictedEntityLookup;
$this->restrictedEntityLookupFactory = $restrictedEntityLookupFactory;
$this->entityAccessLimit = $entityAccessLimit;
}

public static function factory(
RestrictedEntityLookup $restrictedEntityLookup,
RestrictedEntityLookupFactory $restrictedEntityLookupFactory,
SettingsArray $clientSettings
): self {
return new self(
$restrictedEntityLookup,
$restrictedEntityLookupFactory,
$clientSettings->getSetting( 'entityAccessLimit' )
);
}
Expand All @@ -53,7 +50,7 @@ public static function factory(
*/
public function onParserClearState( $parser ) {
// Reset the entity access limits, per T127462
$this->restrictedEntityLookup->reset();
$this->restrictedEntityLookupFactory->getRestrictedEntityLookup( $parser )->reset();
}

/**
Expand All @@ -64,7 +61,7 @@ public function onParserLimitReportPrepare( $parser, $parserOutput ) {
$parserOutput->setLimitReportData(
'limitreport-entityaccesscount',
[
$this->restrictedEntityLookup->getEntityAccessCount(),
$this->restrictedEntityLookupFactory->getRestrictedEntityLookup( $parser )->getEntityAccessCount(),
$this->entityAccessLimit,
]
);
Expand Down
6 changes: 3 additions & 3 deletions client/includes/WikibaseClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
use Wikibase\DataModel\Services\EntityId\EntityIdComposer;
use Wikibase\DataModel\Services\Lookup\EntityLookup;
use Wikibase\DataModel\Services\Lookup\PropertyDataTypeLookup;
use Wikibase\DataModel\Services\Lookup\RestrictedEntityLookup;
use Wikibase\DataModel\Services\Lookup\RestrictedEntityLookupFactory;
use Wikibase\DataModel\Services\Lookup\TermLookup;
use Wikibase\DataModel\Services\Term\PropertyLabelResolver;
use Wikibase\DataModel\Services\Term\TermBuffer;
Expand Down Expand Up @@ -446,9 +446,9 @@ public static function getTermsLanguages( ContainerInterface $services = null ):
->get( 'WikibaseClient.TermsLanguages' );
}

public static function getRestrictedEntityLookup( ContainerInterface $services = null ): RestrictedEntityLookup {
public static function getRestrictedEntityLookupFactory( ContainerInterface $services = null ): RestrictedEntityLookupFactory {
return ( $services ?: MediaWikiServices::getInstance() )
->get( 'WikibaseClient.RestrictedEntityLookup' );
->get( 'WikibaseClient.RestrictedEntityLookupFactory' );
}

public static function getPropertyOrderProvider( ContainerInterface $services = null ): PropertyOrderProvider {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,15 @@ protected function setUp(): void {

public function testStateCleared() {
$title = Title::newMainPage();
$restrictedEntityLookup = WikibaseClient::getRestrictedEntityLookup();
$restrictedEntityLookupFactory = WikibaseClient::getRestrictedEntityLookupFactory();

$popt = new ParserOptions( User::newFromId( 0 ), $this->getServiceContainer()->getLanguageFactory()->getLanguage( 'en' ) );

$parser = $this->getServiceContainer()->getParserFactory()->create();
$parser->parse( '{{#property:P1234|from=Q1}}', $title, $popt );

$restrictedEntityLookup = $restrictedEntityLookupFactory->getRestrictedEntityLookup( $parser );

$this->assertSame( 1, $restrictedEntityLookup->getEntityAccessCount() );

$parser->parse( '{{#property:P1234|from=Q2}}', $title, $popt );
Expand Down
Loading

0 comments on commit c54814b

Please sign in to comment.