jenkins-bot has submitted this change and it was merged. Change subject: Use LabelDescriptionLookup in HistoryEntityAction ......................................................................
Use LabelDescriptionLookup in HistoryEntityAction Bug: T135598 Change-Id: I6d271fc9db69785091e3dedd949993d3bd3120fa --- M repo/includes/Actions/HistoryEntityAction.php M repo/includes/Content/ItemHandler.php M repo/includes/Content/PropertyHandler.php M repo/includes/WikibaseRepo.php M repo/tests/phpunit/includes/Actions/HistoryEntityActionTest.php 5 files changed, 141 insertions(+), 69 deletions(-) Approvals: Jonas Kress (WMDE): Looks good to me, approved Thiemo Mättig (WMDE): Looks good to me, but someone else must approve jenkins-bot: Verified diff --git a/repo/includes/Actions/HistoryEntityAction.php b/repo/includes/Actions/HistoryEntityAction.php index 34275f0..3cff506 100644 --- a/repo/includes/Actions/HistoryEntityAction.php +++ b/repo/includes/Actions/HistoryEntityAction.php @@ -3,9 +3,10 @@ namespace Wikibase; use HistoryAction; -use MWContentSerializationException; -use Wikibase\DataModel\Term\LabelsProvider; -use Wikibase\Repo\WikibaseRepo; +use IContextSource; +use Page; +use Wikibase\DataModel\Services\Lookup\LabelDescriptionLookup; +use Wikibase\Store\EntityIdLookup; /** * Handles the history action for Wikibase entities. @@ -14,52 +15,38 @@ * * @license GPL-2.0+ * @author John Erling Blad < [email protected] > + * @author Adrian Heine <[email protected]> */ class HistoryEntityAction extends HistoryAction { /** - * @var LanguageFallbackChain + * @var EntityIdLookup $entityIdLookup */ - protected $languageFallbackChain; + private $entityIdLookup; /** - * Get the language fallback chain for current context. - * - * @since 0.4 - * - * @return LanguageFallbackChain + * @var LabelDescriptionLookup $labelDescriptionLookup */ - public function getLanguageFallbackChain() { - if ( $this->languageFallbackChain === null ) { - $this->languageFallbackChain = WikibaseRepo::getDefaultInstance()->getLanguageFallbackChainFactory() - ->newFromContext( $this->getContext() ); - } - - return $this->languageFallbackChain; - } + private $labelDescriptionLookup; /** - * Set language fallback chain. + * @since 0.5 * - * @since 0.4 - * - * @param LanguageFallbackChain $chain + * @param Page $page + * @param IContextSource|null $context + * @param EntityIdLookup $entityIdLookup + * @param LabelDescriptionLookup $labelDescriptionLookup */ - public function setLanguageFallbackChain( LanguageFallbackChain $chain ) { - $this->languageFallbackChain = $chain; - } + public function __construct( + Page $page, + IContextSource $context = null, + EntityIdLookup $entityIdLookup, + LabelDescriptionLookup $labelDescriptionLookup + ) { + parent::__construct( $page, $context ); - /** - * Returns the content of the page being viewed. - * - * @return EntityContent|null - */ - protected function getContent() { - try { - return $this->getArticle()->getPage()->getContent(); - } catch ( MWContentSerializationException $ex ) { - return null; - } + $this->entityIdLookup = $entityIdLookup; + $this->labelDescriptionLookup = $labelDescriptionLookup; } /** @@ -68,33 +55,17 @@ * @return string */ protected function getPageTitle() { - $content = $this->getContent(); + $entityId = $this->entityIdLookup->getEntityIdForTitle( $this->getTitle() ); - if ( !$content ) { - // Page does not exist or the entity or redirect can not be deserialized. + if ( !$entityId ) { return parent::getPageTitle(); } - if ( $content->isRedirect() ) { - //TODO: use a message like <autoredircomment> to represent the redirect. - return parent::getPageTitle(); - } + $idSerialization = $entityId->getSerialization(); + $label = $this->labelDescriptionLookup->getLabel( $entityId ); - $entity = $content->getEntity(); - $idSerialization = $entity->getId()->getSerialization(); - $labelText = null; - - if ( $entity instanceof LabelsProvider ) { - $labelData = $this->getLanguageFallbackChain()->extractPreferredValueOrAny( - $entity->getLabels()->toTextArray() - ); - - if ( $labelData ) { - $labelText = $labelData['value']; - } - } - - if ( $labelText !== null ) { + if ( $label !== null ) { + $labelText = $label->getText(); // Escaping HTML characters in order to retain original label that may contain HTML // characters. This prevents having characters evaluated or stripped via // OutputPage::setPageTitle: diff --git a/repo/includes/Content/ItemHandler.php b/repo/includes/Content/ItemHandler.php index cb51f8f..a9be669 100644 --- a/repo/includes/Content/ItemHandler.php +++ b/repo/includes/Content/ItemHandler.php @@ -3,6 +3,8 @@ namespace Wikibase\Repo\Content; use DataUpdate; +use IContextSource; +use Page; use Title; use Wikibase\DataModel\Entity\EntityId; use Wikibase\DataModel\Entity\Item; @@ -13,10 +15,12 @@ use Wikibase\HistoryEntityAction; use Wikibase\ItemContent; use Wikibase\Lib\Store\EntityContentDataCodec; +use Wikibase\Lib\Store\LanguageFallbackLabelDescriptionLookupFactory; use Wikibase\Lib\Store\SiteLinkStore; use Wikibase\Repo\Store\EntityPerPage; use Wikibase\Repo\Validators\EntityConstraintProvider; use Wikibase\Repo\Validators\ValidatorErrorLocalizer; +use Wikibase\Store\EntityIdLookup; use Wikibase\SubmitItemAction; use Wikibase\TermIndex; use Wikibase\Updates\DataUpdateAdapter; @@ -30,6 +34,7 @@ * @license GPL-2.0+ * @author Jeroen De Dauw < [email protected] > * @author Daniel Kinzler + * @author Adrian Heine <[email protected]> */ class ItemHandler extends EntityHandler { @@ -39,6 +44,16 @@ private $siteLinkStore; /** + * @var EntityIdLookup + */ + private $entityIdLookup; + + /** + * @var LanguageFallbackLabelDescriptionLookupFactory + */ + private $labelDescriptionLookupFactory; + + /** * @param EntityPerPage $entityPerPage * @param TermIndex $termIndex * @param EntityContentDataCodec $contentCodec @@ -46,6 +61,8 @@ * @param ValidatorErrorLocalizer $errorLocalizer * @param EntityIdParser $entityIdParser * @param SiteLinkStore $siteLinkStore + * @param EntityIdLookup $entityIdLookup + * @param LanguageFallbackLabelDescriptionLookupFactory $labelDescriptionLookupFactory * @param callable|null $legacyExportFormatDetector */ public function __construct( @@ -56,6 +73,8 @@ ValidatorErrorLocalizer $errorLocalizer, EntityIdParser $entityIdParser, SiteLinkStore $siteLinkStore, + EntityIdLookup $entityIdLookup, + LanguageFallbackLabelDescriptionLookupFactory $labelDescriptionLookupFactory, $legacyExportFormatDetector = null ) { parent::__construct( @@ -69,6 +88,8 @@ $legacyExportFormatDetector ); + $this->entityIdLookup = $entityIdLookup; + $this->labelDescriptionLookupFactory = $labelDescriptionLookupFactory; $this->siteLinkStore = $siteLinkStore; } @@ -88,7 +109,14 @@ */ public function getActionOverrides() { return array( - 'history' => HistoryEntityAction::class, + 'history' => function( Page $page, IContextSource $context = null ) { + return new HistoryEntityAction( + $page, + $context, + $this->entityIdLookup, + $this->labelDescriptionLookupFactory->newLabelDescriptionLookup( $context->getLanguage() ) + ); + }, 'view' => ViewItemAction::class, 'edit' => EditItemAction::class, 'submit' => SubmitItemAction::class, diff --git a/repo/includes/Content/PropertyHandler.php b/repo/includes/Content/PropertyHandler.php index 6f5723b..1a5b6e0 100644 --- a/repo/includes/Content/PropertyHandler.php +++ b/repo/includes/Content/PropertyHandler.php @@ -3,6 +3,8 @@ namespace Wikibase\Repo\Content; use DataUpdate; +use IContextSource; +use Page; use Title; use Wikibase\DataModel\Entity\EntityId; use Wikibase\DataModel\Entity\EntityIdParser; @@ -12,12 +14,14 @@ use Wikibase\EntityContent; use Wikibase\HistoryEntityAction; use Wikibase\Lib\Store\EntityContentDataCodec; +use Wikibase\Lib\Store\LanguageFallbackLabelDescriptionLookupFactory; use Wikibase\PropertyContent; use Wikibase\PropertyInfoBuilder; use Wikibase\PropertyInfoStore; use Wikibase\Repo\Store\EntityPerPage; use Wikibase\Repo\Validators\EntityConstraintProvider; use Wikibase\Repo\Validators\ValidatorErrorLocalizer; +use Wikibase\Store\EntityIdLookup; use Wikibase\SubmitPropertyAction; use Wikibase\TermIndex; use Wikibase\Updates\DataUpdateAdapter; @@ -45,6 +49,16 @@ private $propertyInfoBuilder; /** + * @var EntityIdLookup + */ + private $entityIdLookup; + + /** + * @var LanguageFallbackLabelDescriptionLookupFactory + */ + private $labelDescriptionLookupFactory; + + /** * @see EntityHandler::getContentClass * * @since 0.3 @@ -62,6 +76,8 @@ * @param EntityConstraintProvider $constraintProvider * @param ValidatorErrorLocalizer $errorLocalizer * @param EntityIdParser $entityIdParser + * @param EntityIdLookup $entityIdLookup + * @param LanguageFallbackLabelDescriptionLookupFactory $labelDescriptionLookupFactory * @param PropertyInfoStore $infoStore * @param PropertyInfoBuilder $propertyInfoBuilder * @param callable|null $legacyExportFormatDetector @@ -73,6 +89,8 @@ EntityConstraintProvider $constraintProvider, ValidatorErrorLocalizer $errorLocalizer, EntityIdParser $entityIdParser, + EntityIdLookup $entityIdLookup, + LanguageFallbackLabelDescriptionLookupFactory $labelDescriptionLookupFactory, PropertyInfoStore $infoStore, PropertyInfoBuilder $propertyInfoBuilder, $legacyExportFormatDetector = null @@ -88,6 +106,8 @@ $legacyExportFormatDetector ); + $this->entityIdLookup = $entityIdLookup; + $this->labelDescriptionLookupFactory = $labelDescriptionLookupFactory; $this->infoStore = $infoStore; $this->propertyInfoBuilder = $propertyInfoBuilder; } @@ -97,7 +117,14 @@ */ public function getActionOverrides() { return array( - 'history' => HistoryEntityAction::class, + 'history' => function( Page $page, IContextSource $context = null ) { + return new HistoryEntityAction( + $page, + $context, + $this->entityIdLookup, + $this->labelDescriptionLookupFactory->newLabelDescriptionLookup( $context->getLanguage() ) + ); + }, 'view' => ViewPropertyAction::class, 'edit' => EditPropertyAction::class, 'submit' => SubmitPropertyAction::class, diff --git a/repo/includes/WikibaseRepo.php b/repo/includes/WikibaseRepo.php index eda7fb9..4345aa2 100644 --- a/repo/includes/WikibaseRepo.php +++ b/repo/includes/WikibaseRepo.php @@ -1373,6 +1373,8 @@ $errorLocalizer, $this->getEntityIdParser(), $siteLinkStore, + $this->getEntityIdLookup(), + $this->getLanguageFallbackLabelDescriptionLookupFactory(), $legacyFormatDetector ); @@ -1399,6 +1401,8 @@ $constraintProvider, $errorLocalizer, $this->getEntityIdParser(), + $this->getEntityIdLookup(), + $this->getLanguageFallbackLabelDescriptionLookupFactory(), $propertyInfoStore, $propertyInfoBuilder, $legacyFormatDetector diff --git a/repo/tests/phpunit/includes/Actions/HistoryEntityActionTest.php b/repo/tests/phpunit/includes/Actions/HistoryEntityActionTest.php index 9de9c44..ca88986 100644 --- a/repo/tests/phpunit/includes/Actions/HistoryEntityActionTest.php +++ b/repo/tests/phpunit/includes/Actions/HistoryEntityActionTest.php @@ -12,7 +12,11 @@ use Title; use User; use WebRequest; +use Wikibase\DataModel\Entity\ItemId; +use Wikibase\DataModel\Services\Lookup\LabelDescriptionLookup; +use Wikibase\DataModel\Term\Term; use Wikibase\HistoryEntityAction; +use Wikibase\Store\EntityIdLookup; /** * @covers Wikibase\HistoryEntityAction @@ -24,6 +28,7 @@ * * @license GPL-2.0+ * @author Thiemo Mättig + * @author Adrian Heine <[email protected]> */ class HistoryEntityActionTest extends PHPUnit_Framework_TestCase { @@ -39,6 +44,10 @@ $page->expects( $this->any() ) ->method( 'getTitle' ) ->will( $this->returnValue( Title::newFromText( $title ) ) ); + $page->expects( $this->never() ) + ->method( 'getPage' ) + // Deserializing the full entity may fail, see https://gerrit.wikimedia.org/r/262881 + ->will( $this->throwException( new MWContentSerializationException() ) ); return $page; } @@ -63,10 +72,10 @@ $context = $this->getMock( IContextSource::class ); $context->expects( $this->once() ) ->method( 'getConfig' ) - ->will( $this->returnValue( new HashConfig( array( + ->will( $this->returnValue( new HashConfig( [ 'UseFileCache' => false, 'UseMediaWikiUIEverywhere' => false, - ) ) ) ); + ] ) ) ); $context->expects( $this->any() ) ->method( 'getRequest' ) ->will( $this->returnValue( new WebRequest() ) ); @@ -90,18 +99,51 @@ return $context; } - public function testGivenUnDeserializableRevision_historyActionDoesNotFail() { - $page = $this->getPage( 'Page title' ); - $page->expects( $this->once() ) - ->method( 'getPage' ) - ->will( $this->throwException( new MWContentSerializationException() ) ); + public function pageTitleProvider() { + return [ + 'fallback to parent' => [ + null, + null, + '(history-title: Page title)' + ], + 'without label' => [ + new ItemId( 'Q1' ), + null, + '(wikibase-history-title-without-label: Q1)' + ], + 'with label' => [ + new ItemId( 'Q2' ), + new Term( 'qqx', 'Label' ), + '(wikibase-history-title-with-label: Q2, Label)' + ], + ]; + } + + /** + * @dataProvider pageTitleProvider + */ + public function testGetPageTitle( ItemId $entityId = null, Term $label = null, $expected ) { + $entityIdLookup = $this->getMock( EntityIdLookup::class ); + $entityIdLookup->expects( $this->once() ) + ->method( 'getEntityIdForTitle' ) + ->will( $this->returnValue( $entityId ) ); + + $labelLookup = $this->getMock( LabelDescriptionLookup::class ); + $labelLookup->expects( $this->any() ) + ->method( 'getLabel' ) + ->will( $this->returnValue( $label ) ); $output = $this->getOutput(); $output->expects( $this->once() ) ->method( 'setPageTitle' ) - ->with( '(history-title: Page title)' ); + ->with( $expected ); - $action = new HistoryEntityAction( $page, $this->getContext( $output ) ); + $action = new HistoryEntityAction( + $this->getPage( 'Page title' ), + $this->getContext( $output ), + $entityIdLookup, + $labelLookup + ); $action->show(); } -- To view, visit https://gerrit.wikimedia.org/r/289377 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I6d271fc9db69785091e3dedd949993d3bd3120fa Gerrit-PatchSet: 2 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Adrian Heine <[email protected]> Gerrit-Reviewer: Aude <[email protected]> Gerrit-Reviewer: Daniel Kinzler <[email protected]> Gerrit-Reviewer: Jonas Kress (WMDE) <[email protected]> Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
