Bene has uploaded a new change for review.
https://gerrit.wikimedia.org/r/271500
Change subject: Fix bug in EntityParserOutputDataUpdater
......................................................................
Fix bug in EntityParserOutputDataUpdater
This fixes an actual bug in the data updater class which doesn't register
SiteLinkDataUpdater instances if they are also an instance of
StatementDataUpdater.
By fixing this, we can get rid of the special handling for badges in
EntityParserOutputGenerator as it only duplicates the work done in the
EntityParserOutputDataUpdater and ReferencedEntitiesDataUpdater.
Change-Id: I869e73324eca73b2cc9396bf27db0774feb02c48
---
M repo/includes/ParserOutput/EntityParserOutputDataUpdater.php
M repo/includes/ParserOutput/EntityParserOutputGenerator.php
M repo/tests/phpunit/includes/ParserOutput/EntityParserOutputDataUpdaterTest.php
M repo/tests/phpunit/includes/ParserOutput/EntityParserOutputGeneratorTest.php
4 files changed, 57 insertions(+), 31 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/00/271500/1
diff --git a/repo/includes/ParserOutput/EntityParserOutputDataUpdater.php
b/repo/includes/ParserOutput/EntityParserOutputDataUpdater.php
index 4e36729..2b634b7 100644
--- a/repo/includes/ParserOutput/EntityParserOutputDataUpdater.php
+++ b/repo/includes/ParserOutput/EntityParserOutputDataUpdater.php
@@ -49,20 +49,31 @@
*/
public function __construct( ParserOutput $parserOutput, array
$dataUpdaters ) {
foreach ( $dataUpdaters as $updater ) {
- if ( $updater instanceof StatementDataUpdater ) {
- $this->statementDataUpdaters[] = $updater;
- } elseif ( $updater instanceof SiteLinkDataUpdater ) {
- $this->siteLinkDataUpdaters[] = $updater;
- } else {
- throw new InvalidArgumentException( 'Each
$dataUpdaters element must be a '
- . 'StatementDataUpdater,
SiteLinkDataUpdater or both' );
- }
+ $this->registerDataUpdater( $updater );
}
$this->parserOutput = $parserOutput;
$this->dataUpdaters = $dataUpdaters;
}
+ private function registerDataUpdater( $updater ) {
+ if ( !( $updater instanceof StatementDataUpdater ) &&
+ !( $updater instanceof SiteLinkDataUpdater )
+ ) {
+ throw new InvalidArgumentException(
+ 'Each $dataUpdaters element must be a
StatementDataUpdater, SiteLinkDataUpdater or both'
+ );
+ }
+
+ if ( $updater instanceof StatementDataUpdater ) {
+ $this->statementDataUpdaters[] = $updater;
+ }
+
+ if ( $updater instanceof SiteLinkDataUpdater ) {
+ $this->siteLinkDataUpdaters[] = $updater;
+ }
+ }
+
/**
* @param EntityDocument $entity
*/
diff --git a/repo/includes/ParserOutput/EntityParserOutputGenerator.php
b/repo/includes/ParserOutput/EntityParserOutputGenerator.php
index c2b9bd8..3486eaa 100644
--- a/repo/includes/ParserOutput/EntityParserOutputGenerator.php
+++ b/repo/includes/ParserOutput/EntityParserOutputGenerator.php
@@ -158,13 +158,7 @@
$configVars = $this->configBuilder->build( $entity );
$parserOutput->addJsConfigVars( $configVars );
-
- // FIXME: OCP violation -
https://phabricator.wikimedia.org/T75495
- if ( $entity instanceof Item ) {
- $this->addBadgesToParserOutput( $parserOutput,
$entity->getSiteLinkList() );
- }
-
- $this->addTitleTextToParserOutput( $parserOutput, $entity );
+ $parserOutput->setExtensionData( 'wikibase-titletext',
$this->getTitleText( $entity ) );
if ( $generateHtml ) {
$this->addHtmlToParserOutput(
@@ -238,23 +232,11 @@
}
/**
- * @param ParserOutput $parserOutput
- * @param SiteLinkList $siteLinkList
- */
- private function addBadgesToParserOutput( ParserOutput $parserOutput,
SiteLinkList $siteLinkList ) {
- /** @var SiteLink $siteLink */
- foreach ( $siteLinkList as $siteLink ) {
- foreach ( $siteLink->getBadges() as $badge ) {
- $parserOutput->addLink(
$this->entityTitleLookup->getTitleForId( $badge ) );
- }
- }
- }
-
- /**
- * @param ParserOutput $parserOutput
* @param EntityDocument $entity
+ *
+ * @return string
*/
- private function addTitleTextToParserOutput( ParserOutput
$parserOutput, EntityDocument $entity ) {
+ private function getTitleText( EntityDocument $entity ) {
$titleText = null;
if ( $entity instanceof FingerprintProvider ) {
@@ -274,7 +256,7 @@
}
}
- $parserOutput->setExtensionData( 'wikibase-titletext',
$titleText );
+ return $titleText;
}
/**
diff --git
a/repo/tests/phpunit/includes/ParserOutput/EntityParserOutputDataUpdaterTest.php
b/repo/tests/phpunit/includes/ParserOutput/EntityParserOutputDataUpdaterTest.php
index 1502ee2..713e32a 100644
---
a/repo/tests/phpunit/includes/ParserOutput/EntityParserOutputDataUpdaterTest.php
+++
b/repo/tests/phpunit/includes/ParserOutput/EntityParserOutputDataUpdaterTest.php
@@ -37,13 +37,26 @@
$siteLinkDataUpdater->expects( $this->once() )
->method( 'updateParserOutput' );
+ $siteLinkAndStatementDataUpdater = $this->getMockBuilder(
'Wikibase\Repo\ParserOutput\ReferencedEntitiesDataUpdater' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $siteLinkAndStatementDataUpdater->expects( $this->exactly(
$statements ) )
+ ->method( 'processStatement' );
+ $siteLinkAndStatementDataUpdater->expects( $this->exactly(
$siteLinks ) )
+ ->method( 'processSiteLink' );
+ $siteLinkAndStatementDataUpdater->expects( $this->once() )
+ ->method( 'updateParserOutput' );
+
$instance = new EntityParserOutputDataUpdater( new
ParserOutput(), array(
$statementDataUpdater,
$siteLinkDataUpdater,
+ $siteLinkAndStatementDataUpdater
) );
+
foreach ( $entities as $entity ) {
$instance->processEntity( $entity );
}
+
$instance->finish();
}
diff --git
a/repo/tests/phpunit/includes/ParserOutput/EntityParserOutputGeneratorTest.php
b/repo/tests/phpunit/includes/ParserOutput/EntityParserOutputGeneratorTest.php
index 9f4d46e..937d399 100644
---
a/repo/tests/phpunit/includes/ParserOutput/EntityParserOutputGeneratorTest.php
+++
b/repo/tests/phpunit/includes/ParserOutput/EntityParserOutputGeneratorTest.php
@@ -75,6 +75,23 @@
'images'
);
+ // TODO would be nice to test this, but
ReferencedEntitiesDataUpdater uses LinkBatch which uses the database
+// $this->assertEquals(
+// array( 'item:Q42', 'item:Q35' ),
+// array_keys( $parserOutput->getLinks()[NS_MAIN] ),
+// 'badges'
+// );
+
+ $this->assertArrayEquals(
+ array(
+ new ItemId( 'Q42' ),
+ new ItemId( 'Q35' ),
+ new PropertyId( 'P42' ),
+ new PropertyId( 'P10' )
+ ),
+ $parserOutput->getExtensionData( 'referenced-entities' )
+ );
+
$expectedUsedOptions = array( 'userlang', 'editsection' );
$actualOptions = $parserOutput->getUsedOptions();
$missingOptions = array_diff( $expectedUsedOptions,
$actualOptions );
@@ -203,6 +220,9 @@
$statements->addNewStatement( new PropertyValueSnak( 10, new
StringValue( 'File:This is a file.pdf' ) ) );
$statements->addNewStatement( new PropertyValueSnak( 10, new
StringValue( 'File:Selfie.jpg' ) ) );
+ $item->getSiteLinkList()->addNewSiteLink( 'enwiki', 'kitten',
array( new ItemId( 'Q42' ) ) );
+ $item->getSiteLinkList()->addNewSiteLink( 'dewiki', 'meow',
array( new ItemId( 'Q42' ), new ItemId( 'Q35' ) ) );
+
return $item;
}
--
To view, visit https://gerrit.wikimedia.org/r/271500
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I869e73324eca73b2cc9396bf27db0774feb02c48
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Bene <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
