Aude has uploaded a new change for review.
https://gerrit.wikimedia.org/r/176804
Change subject: Use TermLookup in LinkBegin hook
......................................................................
Use TermLookup in LinkBegin hook
(aude) minimally changes the tests here and then
make test refactoring a follow up patch.
Bug: T72767
Change-Id: I8b696c91f8f5c0ddef3df29a82a1f97206b2eb91
---
M repo/includes/Hook/LinkBeginHookHandler.php
M repo/includes/WikibaseRepo.php
M repo/includes/content/EntityContentFactory.php
A repo/includes/store/PageEntityIdLookup.php
M repo/tests/phpunit/includes/Hook/LinkBeginHookHandlerTest.php
M repo/tests/phpunit/includes/content/EntityContentFactoryTest.php
6 files changed, 221 insertions(+), 116 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/04/176804/1
diff --git a/repo/includes/Hook/LinkBeginHookHandler.php
b/repo/includes/Hook/LinkBeginHookHandler.php
index 39160a5..e883743 100644
--- a/repo/includes/Hook/LinkBeginHookHandler.php
+++ b/repo/includes/Hook/LinkBeginHookHandler.php
@@ -4,16 +4,14 @@
use DummyLinker;
use Html;
-use IContextSource;
use Language;
-use MWContentSerializationException;
+use OutputPage;
use RequestContext;
use Title;
-use Wikibase\EntityContent;
-use Wikibase\LanguageFallbackChainFactory;
-use Wikibase\Repo\Content\EntityContentFactory;
+use Wikibase\LanguageFallbackChain;
+use Wikibase\Lib\Store\TermLookup;
+use Wikibase\Repo\Store\PageEntityIdLookup;
use Wikibase\Repo\WikibaseRepo;
-use WikiPage;
/**
* @since 0.5
@@ -23,29 +21,38 @@
class LinkBeginHookHandler {
/**
- * @var EntityContentFactory
+ * @var PageEntityIdLookup
*/
- private $entityContentFactory;
+ private $entityIdLookup;
/**
- * @var LanguageFallbackChainFactory
+ * @var TermLookup
*/
- private $languageFallbackChainFactory;
+ private $termLookup;
/**
- * @var IContextSource
+ * @var LanguageFallbackChain
*/
- private $context;
+ private $languageFallback;
+
+ /**
+ * @var Language
+ */
+ private $pageLanguage;
/**
* @return LinkBeginHookHandler
*/
- public static function newFromGlobalState() {
- $entityContentFactory =
WikibaseRepo::getDefaultInstance()->getEntityContentFactory();
- $languageFallbackChainFactory =
WikibaseRepo::getDefaultInstance()->getLanguageFallbackChainFactory();
+ private static function newFromGlobalState() {
$context = RequestContext::getMain();
- return new self( $entityContentFactory,
$languageFallbackChainFactory, $context );
+ $entityIdLookup =
WikibaseRepo::getDefaultInstance()->getPageEntityIdLookup();
+ $termLookup =
WikibaseRepo::getDefaultInstance()->getTermLookup();
+
+ $languageFallbackChainFactory =
WikibaseRepo::getDefaultInstance()->getLanguageFallbackChainFactory();
+ $languageFallbackChain =
$languageFallbackChainFactory->newFromContext( $context );
+
+ return new self( $entityIdLookup, $termLookup,
$languageFallbackChain, $context->getLanguage() );
}
/**
@@ -66,120 +73,126 @@
&$options, &$ret
) {
$handler = self::newFromGlobalState();
- return $handler->doOnLinkBegin( $target, $html, $customAttribs
);
+ $context = RequestContext::getMain();
+
+ $handler->doOnLinkBegin( $target, $html, $customAttribs,
$context->getOutput() );
+
+ return true;
}
/**
- * @param EntityContentFactory $entityContentFactory
- * @param LanguageFallbackChainFactory $languageFallbackChainFactory
- * @param IContextSource $context
+ * @param PageEntityIdLookup $entityIdLookup
+ * @param TermLookup $termLookup
+ * @param LanguageFallbackChain $languageFallback
+ * @param Language $pageLanguage
+ *
+ * @todo: Would be nicer to take a LabelLookup instead of TermLookup +
FallbackChain.
+ * But LabelLookup does not support descriptions at the moment.
*/
public function __construct(
- EntityContentFactory $entityContentFactory,
- LanguageFallbackChainFactory $languageFallbackChainFactory,
- IContextSource $context
+ PageEntityIdLookup $entityIdLookup,
+ TermLookup $termLookup,
+ LanguageFallbackChain $languageFallback,
+ Language $pageLanguage
) {
- $this->entityContentFactory = $entityContentFactory;
- $this->languageFallbackChainFactory =
$languageFallbackChainFactory;
- $this->context = $context;
+ $this->entityIdLookup = $entityIdLookup;
+ $this->termLookup = $termLookup;
+ $this->languageFallback = $languageFallback;
+ $this->pageLanguage = $pageLanguage;
}
/**
* @param Title $target
* @param string &$html
* @param array &$customAttribs
- *
- * @return boolean
+ * @param OutputPage $out
*/
- public function doOnLinkBegin( Title $target, &$html, array
&$customAttribs ) {
+ public function doOnLinkBegin( Title $target, &$html, array
&$customAttribs, OutputPage $out ) {
wfProfileIn( __METHOD__ );
- if ( !$this->onSpecialPage() || !$this->isEntityContent(
$target ) ) {
+ $currentTitle = $out->getTitle();
+
+ if ( $currentTitle === null || !$currentTitle->isSpecialPage()
) {
+ // Note: this may not work right with special page
transclusion. If $out->getTitle()
+ // doesn't return the transcluded special page's title,
the transcluded text will
+ // not have entity IDs resolved to labels.
wfProfileOut( __METHOD__ );
- return true;
+ return;
}
// if custom link text is given, there is no point in
overwriting it
// but not if it is similar to the plain title
if ( $html !== null && $target->getFullText() !== $html ) {
wfProfileOut( __METHOD__ );
- return true;
+ return;
}
- $entity = $this->getEntityForTitle( $target );
+ $entityId = $this->entityIdLookup->getPageEntityId( $target );
- if ( !$entity ) {
+ if ( !$entityId ) {
wfProfileOut( __METHOD__ );
- return true;
+ return;
}
- // Try to find the most preferred available language to display
data in current context.
- $languageFallbackChain =
$this->languageFallbackChainFactory->newFromContext( $this->context );
+ //@todo: only fetch the labels we need for the fallback chain
+ $labels = $this->termLookup->getLabels( $entityId );
+ $descriptions = $this->termLookup->getDescriptions( $entityId );
- $labelData =
$languageFallbackChain->extractPreferredValueOrAny( $entity->getLabels() );
+ $labelData = $this->getPreferredTerm( $labels );
+ $descriptionData = $this->getPreferredTerm( $descriptions );
- if ( $labelData ) {
- $labelText = $labelData['value'];
- $labelLang = Language::factory( $labelData['language']
);
- } else {
- $labelText = '';
- $labelLang = $this->context->getLanguage();
- }
-
- $descriptionData =
$languageFallbackChain->extractPreferredValueOrAny(
- $entity->getDescriptions()
- );
-
- $html = $this->getHtml( $target, $labelLang, $labelText );
+ $html = $this->getHtml( $target, $labelData );
$customAttribs['title'] = $this->getTitleAttribute(
$target,
- $labelLang,
- $labelText,
+ $labelData,
$descriptionData
);
// add wikibase styles in all cases, so we can format the link
properly:
- $this->context->getOutput()->addModuleStyles( array(
'wikibase.common' ) );
+ $out->addModuleStyles( array( 'wikibase.common' ) );
wfProfileOut( __METHOD__ );
- return true;
}
- private function onSpecialPage() {
- // Title is temporarily set to special pages Title in case of
special page inclusion!
- // Therefore we can just check whether the page is a special
page and
- // if not, disable the behavior.
- $currentTitle = $this->context->getTitle();
+ private function getPreferredTerm( $termsByLanguage ) {
+ if ( empty( $termsByLanguage ) ) {
+ return null;
+ }
- return $currentTitle !== null && $currentTitle->isSpecialPage();
+ return $this->languageFallback->extractPreferredValueOrAny(
+ $termsByLanguage
+ );
}
- private function isEntityContent( Title $title ) {
- return $this->entityContentFactory->isEntityContentModel(
$title->getContentModel() );
- }
-
- private function getEntityForTitle( Title $title ) {
- $page = new WikiPage( $title );
-
- try {
- $content = $page->getContent();
-
- // Failed, can't continue. This could happen because
the content is empty
- // (page doesn't exist), e.g. after item was deleted.
-
- // TODO: resolve redirect, show redirect info in link
- if ( $content instanceof EntityContent &&
!$content->isRedirect() ) {
- return $content->getEntity();
- }
- } catch ( MWContentSerializationException $ex ) {
- // if this fails, it's not horrible.
- wfWarn( 'Failed to get entity object for [[' .
$title->getFullText() . ']]'
- . ': ' . $ex->getMessage() );
+ /**
+ * @param array $termData A term record as returned by
+ * LanguageFallbackChain::extractPreferredValueOrAny(),
+ * containing the 'value' and 'language' fields, or null
+ * or an empty array.
+ *
+ * @see LanguageFallbackChain::extractPreferredValueOrAny
+ *
+ * @return array list( string $text, Language $language )
+ */
+ private function extractTextAndLanguage( $termData ) {
+ if ( $termData ) {
+ return array(
+ $termData['value'],
+ Language::factory( $termData['language'] )
+ );
+ } else {
+ return array(
+ '',
+ $this->pageLanguage
+ );
}
}
- private function getHtml( Title $title, $labelLang, $labelText ) {
+ private function getHtml( Title $title, $labelData ) {
+ /** @var Language $labelLang */
+ list( $labelText, $labelLang ) = $this->extractTextAndLanguage(
$labelData );
+
$idHtml = Html::openElement( 'span', array( 'class' =>
'wb-itemlink-id' ) )
. wfMessage(
'wikibase-itemlink-id-wrapper',
@@ -203,26 +216,27 @@
. Html::closeElement( 'span' );
}
- private function getTitleAttribute( Title $title, $labelLang,
$labelText, $descriptionData ) {
- if ( $descriptionData ) {
- $descriptionText = $descriptionData['value'];
- $descriptionLang = Language::factory(
$descriptionData['language'] );
- } else {
- $descriptionText = '';
- $descriptionLang = $this->context->getLanguage();
- }
+ private function getTitleAttribute( Title $title, $labelData,
$descriptionData ) {
+ /** @var Language $labelLang */
+ /** @var Language $descriptionLang */
+
+ list( $labelText, $labelLang ) = $this->extractTextAndLanguage(
$labelData );
+ list( $descriptionText, $descriptionLang ) =
$this->extractTextAndLanguage( $descriptionData );
// Set title attribute for constructed link, and make tricks
with the directionality to get it right
$titleText = ( $labelText !== '' )
- ? $labelLang->getDirMark() . $labelText .
$this->context->getLanguage()->getDirMark()
+ ? $labelLang->getDirMark() . $labelText
+ . $this->pageLanguage->getDirMark()
: $title->getPrefixedText();
+
+ $descriptionText = $descriptionLang->getDirMark() .
$descriptionText
+ . $this->pageLanguage->getDirMark();
return ( $descriptionText !== '' ) ?
wfMessage(
'wikibase-itemlink-title',
$titleText,
- $descriptionLang->getDirMark() .
$descriptionText
- .
$this->context->getLanguage()->getDirMark()
+ $descriptionText
)->inContentLanguage()->text() :
$titleText; // no description, just display the title
then
}
diff --git a/repo/includes/WikibaseRepo.php b/repo/includes/WikibaseRepo.php
index 2b89aa0..7ee6e9b 100644
--- a/repo/includes/WikibaseRepo.php
+++ b/repo/includes/WikibaseRepo.php
@@ -67,6 +67,7 @@
use Wikibase\Repo\Notifications\DatabaseChangeTransmitter;
use Wikibase\Repo\Notifications\DummyChangeTransmitter;
use Wikibase\Repo\Store\EntityPermissionChecker;
+use Wikibase\Repo\Store\PageEntityIdLookup;
use Wikibase\Repo\View\EntityViewFactory;
use Wikibase\Settings;
use Wikibase\SettingsArray;
@@ -279,6 +280,15 @@
/**
* @since 0.5
*
+ * @return PageEntityIdLookup
+ */
+ public function getPageEntityIdLookup() {
+ return $this->getEntityContentFactory();
+ }
+
+ /**
+ * @since 0.5
+ *
* @param string $uncached Flag string, set to 'uncached' to get an
uncached direct lookup service.
*
* @return \Wikibase\Lib\Store\EntityRevisionLookup
diff --git a/repo/includes/content/EntityContentFactory.php
b/repo/includes/content/EntityContentFactory.php
index 1bc4454..466449f 100644
--- a/repo/includes/content/EntityContentFactory.php
+++ b/repo/includes/content/EntityContentFactory.php
@@ -8,6 +8,7 @@
use Revision;
use Status;
use Title;
+use TitleValue;
use User;
use Wikibase\DataModel\Entity\Entity;
use Wikibase\DataModel\Entity\EntityId;
@@ -15,6 +16,7 @@
use Wikibase\Lib\Store\EntityRedirect;
use Wikibase\Lib\Store\EntityTitleLookup;
use Wikibase\Repo\Store\EntityPermissionChecker;
+use Wikibase\Repo\Store\PageEntityIdLookup;
/**
* Factory for EntityContent objects.
@@ -25,7 +27,7 @@
* @author Jeroen De Dauw < [email protected] >
* @author Daniel Kinzler
*/
-class EntityContentFactory implements EntityTitleLookup,
EntityPermissionChecker {
+class EntityContentFactory implements EntityTitleLookup, PageEntityIdLookup,
EntityPermissionChecker {
/**
* @since 0.5
@@ -88,6 +90,27 @@
}
/**
+ * Returns the ID of the entity associated with the given page title.
+ *
+ * @note There is no guarantee that the EntityId returned by this
method refers to
+ * an existing entity.
+ *
+ * @param Title $title
+ *
+ * @return EntityId|null
+ */
+ public function getPageEntityId( Title $title ) {
+ $contentModel = $title->getContentModel();
+ $handler = ContentHandler::getForModelID( $contentModel );
+
+ if ( $handler instanceof EntityHandler ) {
+ return $handler->getIdForTitle( $title );
+ } else {
+ return null;
+ }
+ }
+
+ /**
* Determines what namespace is suitable for the given type of entities.
*
* @since 0.5
diff --git a/repo/includes/store/PageEntityIdLookup.php
b/repo/includes/store/PageEntityIdLookup.php
new file mode 100644
index 0000000..aeca200
--- /dev/null
+++ b/repo/includes/store/PageEntityIdLookup.php
@@ -0,0 +1,37 @@
+<?php
+
+namespace Wikibase\Repo\Store;
+
+use Title;
+use Wikibase\DataModel\Entity\EntityId;
+
+/**
+ * Service interface for a lookup that provides access to the ID of the entity
+ * stored on a given page.
+ *
+ * The mapping may be implemented programmatically (e.g. by parsing the title
as an EntityId),
+ * or it may be based on an explicit mapping (e.g. in a database table).
+ *
+ * @since 0.5
+ *
+ * @licence GNU GPL v2+
+ * @author Daniel Kinzler
+ */
+interface PageEntityIdLookup {
+
+ /**
+ * Returns the ID of the entity stored on the page identified by $title.
+ *
+ * @note There is no guarantee that the EntityId returned by this
method refers to
+ * an existing entity.
+ *
+ * @param Title $title
+ *
+ * @todo: Switch this to using TitleValue once we can easily get the
content model and
+ * handler based on a TitleValue.
+ *
+ * @return EntityId|null
+ */
+ public function getPageEntityId( Title $title );
+
+}
diff --git a/repo/tests/phpunit/includes/Hook/LinkBeginHookHandlerTest.php
b/repo/tests/phpunit/includes/Hook/LinkBeginHookHandlerTest.php
index 0d48d8b..2df87b1 100644
--- a/repo/tests/phpunit/includes/Hook/LinkBeginHookHandlerTest.php
+++ b/repo/tests/phpunit/includes/Hook/LinkBeginHookHandlerTest.php
@@ -36,9 +36,9 @@
private static $noLabelItemId = null;
/**
- * @var EntityTitleLookup
+ * @var EntityContentFactory
*/
- private $entityTitleLookup = null;
+ private $entityContentFactory = null;
protected function setUp() {
parent::setUp();
@@ -46,7 +46,7 @@
$language = Language::factory( 'en' );
$this->setMwGlobals( array(
- 'wgLanguageCode' => 'en',
+ 'wgLanguageCode' => 'en',
'wgLang' => $language,
'wgContLang' => $language
) );
@@ -76,12 +76,12 @@
$contextTitle = Title::newFromText( 'Special:Recentchanges' );
$linkBeginHookHandler = $this->getLinkBeginHookHandler(
$contextTitle );
- $title = $this->getEntityTitleLookup()->getTitleForId(
self::$itemId );
+ $title = $this->getEntityContentFactory()->getTitleForId(
self::$itemId );
$html = $title->getFullText();
$customAttribs = array();
- $linkBeginHookHandler->doOnLinkBegin( $title, $html,
$customAttribs );
+ $linkBeginHookHandler->doOnLinkBegin( $title, $html,
$customAttribs, $this->getOutput( $title ) );
$expectedHtml = '<span class="wb-itemlink">'
. '<span class="wb-itemlink-label" lang="en"
dir="ltr">linkbegin-label</span> '
@@ -97,13 +97,13 @@
public function testDoOnLinkBegin_onNonSpecialPage() {
$linkBeginHookHandler = $this->getLinkBeginHookHandler(
Title::newMainPage() );
- $title = $this->getEntityTitleLookup()->getTitleForId(
self::$itemId );
+ $title = $this->getEntityContentFactory()->getTitleForId(
self::$itemId );
$titleText = $title->getFullText();
$html = $titleText;
$customAttribs = array();
- $linkBeginHookHandler->doOnLinkBegin( $title, $html,
$customAttribs );
+ $linkBeginHookHandler->doOnLinkBegin( $title, $html,
$customAttribs, $this->getOutput( $title ) );
$this->assertEquals( $titleText, $html );
$this->assertEquals( array(), $customAttribs );
@@ -119,7 +119,7 @@
$html = $titleText;
$customAttribs = array();
- $linkBeginHookHandler->doOnLinkBegin( $title, $html,
$customAttribs );
+ $linkBeginHookHandler->doOnLinkBegin( $title, $html,
$customAttribs, $this->getOutput( $title ) );
$this->assertEquals( $titleText, $html );
$this->assertEquals( array(), $customAttribs );
@@ -130,13 +130,13 @@
$linkBeginHookHandler = $this->getLinkBeginHookHandler(
$contextTitle );
$itemId = ItemId::newFromNumber( mt_rand( 0, 9999999999 ) );
- $title = $this->getEntityTitleLookup()->getTitleForId( $itemId
);
+ $title = $this->getEntityContentFactory()->getTitleForId(
$itemId );
$titleText = $title->getFullText();
$html = $titleText;
$customAttribs = array();
- $linkBeginHookHandler->doOnLinkBegin( $title, $html,
$customAttribs );
+ $linkBeginHookHandler->doOnLinkBegin( $title, $html,
$customAttribs, $this->getOutput( $title ) );
$this->assertEquals( $titleText, $html );
$this->assertEquals( array(), $customAttribs );
@@ -146,12 +146,12 @@
$contextTitle = Title::newFromText( 'Special:Recentchanges' );
$linkBeginHookHandler = $this->getLinkBeginHookHandler(
$contextTitle );
- $title = $this->getEntityTitleLookup()->getTitleForId(
self::$noLabelItemId );
+ $title = $this->getEntityContentFactory()->getTitleForId(
self::$noLabelItemId );
$html = $title->getFullText();
$customAttribs = array();
- $linkBeginHookHandler->doOnLinkBegin( $title, $html,
$customAttribs );
+ $linkBeginHookHandler->doOnLinkBegin( $title, $html,
$customAttribs, $this->getOutput( $title ) );
$expected = '<span class="wb-itemlink">'
. '<span class="wb-itemlink-label" lang="en"
dir="ltr"></span> '
@@ -163,25 +163,36 @@
$this->assertContains(
self::$noLabelItemId->getSerialization(), $customAttribs['title'] );
}
- private function getEntityTitleLookup() {
- if ( $this->entityTitleLookup === null ) {
+ private function getEntityContentFactory() {
+ if ( $this->entityContentFactory === null ) {
$wikibaseRepo = WikibaseRepo::getDefaultInstance();
- $this->entityTitleLookup =
$wikibaseRepo->getEntityTitleLookup();
+ $this->entityContentFactory =
$wikibaseRepo->getEntityContentFactory();
}
- return $this->entityTitleLookup;
+ return $this->entityContentFactory;
+ }
+
+ private function getOutput( Title $title ) {
+ return $this->getContext( $title )->getOutput();
+ }
+
+ private function getContext( Title $title ) {
+ $context = RequestContext::getMain();
+ $context->setTitle( $title );
+
+ return $context;
}
private function getLinkBeginHookHandler( Title $title ) {
- $context = RequestContext::getMain();
- $context->setTitle( $title );
+ $context = $this->getContext( $title );
$wikibaseRepo = WikibaseRepo::getDefaultInstance();
return new LinkBeginHookHandler(
- $this->getEntityTitleLookup(),
- $wikibaseRepo->getLanguageFallbackChainFactory(),
- $context
+ $wikibaseRepo->getEntityContentFactory(),
+ $wikibaseRepo->getTermLookup(),
+
$wikibaseRepo->getLanguageFallbackChainFactory()->newFromContext( $context ),
+ $context->getLanguage()
);
}
diff --git a/repo/tests/phpunit/includes/content/EntityContentFactoryTest.php
b/repo/tests/phpunit/includes/content/EntityContentFactoryTest.php
index 460cb3f..3bac18a 100644
--- a/repo/tests/phpunit/includes/content/EntityContentFactoryTest.php
+++ b/repo/tests/phpunit/includes/content/EntityContentFactoryTest.php
@@ -3,6 +3,7 @@
namespace Wikibase\Test;
use ContentHandler;
+use Title;
use Wikibase\DataModel\Entity\Entity;
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\ItemId;
@@ -82,6 +83,15 @@
$this->assertEquals( 'Q42', $title->getText() );
}
+ public function testGetPageEntityId() {
+ $factory = $this->newFactory();
+
+ $title = Title::makeTitle( $factory->getNamespaceForType(
Item::ENTITY_TYPE ), 'Q42' );
+ $entityId = $factory->getPageEntityId( $title );
+
+ $this->assertEquals( 'Q42', $entityId->getSerialization() );
+ }
+
public function testGetNamespaceForType() {
$factory = $this->newFactory();
$id = new ItemId( 'Q42' );
--
To view, visit https://gerrit.wikimedia.org/r/176804
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8b696c91f8f5c0ddef3df29a82a1f97206b2eb91
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Aude <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
