Hoo man has uploaded a new change for review. https://gerrit.wikimedia.org/r/171992
Change subject: Don't load the full entity into Lua to get labels/ sitelinks ...................................................................... Don't load the full entity into Lua to get labels/ sitelinks This approach makes sure we have less round tripping between PHP and Lua with large entity arrays, and reduces the memory usage in Lua (especially if a lot of items are loaded). In the future we might want to use a memcached label and sitelink lookup to make things even faster. Bug: 71743 Change-Id: Ic96dca7dc7abce339b1ce05d678db001fd432189 --- M client/includes/scribunto/Scribunto_LuaWikibaseLibrary.php M client/includes/scribunto/WikibaseLuaBindings.php M client/includes/scribunto/mw.wikibase.lua M client/tests/phpunit/includes/scribunto/LuaWikibaseLibraryTests.lua M client/tests/phpunit/includes/scribunto/WikibaseLuaBindingsTest.php 5 files changed, 150 insertions(+), 14 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase refs/changes/92/171992/1 diff --git a/client/includes/scribunto/Scribunto_LuaWikibaseLibrary.php b/client/includes/scribunto/Scribunto_LuaWikibaseLibrary.php index 26a2e41..a546cf3 100644 --- a/client/includes/scribunto/Scribunto_LuaWikibaseLibrary.php +++ b/client/includes/scribunto/Scribunto_LuaWikibaseLibrary.php @@ -4,6 +4,8 @@ use Wikibase\Client\WikibaseClient; use Wikibase\DataModel\Entity\EntityIdParsingException; use Wikibase\Utils; +use Wikibase\Lib\Store\LanguageLabelLookup; +use Wikibase\Lib\Store\EntityRetrievingTermLookup; /** * Registers and defines functions to access Wikibase through the Scribunto extension @@ -34,14 +36,21 @@ $wikibaseClient = WikibaseClient::getDefaultInstance(); + $entityLookup = $wikibaseClient->getStore()->getEntityLookup(); + $labelLookup = new LanguageLabelLookup( + new EntityRetrievingTermLookup( $entityLookup ), + $wgContLang->getCode() + ); + $this->wbLibrary = new WikibaseLuaBindings( $wikibaseClient->getEntityIdParser(), - $wikibaseClient->getStore()->getEntityLookup(), + $entityLookup, $wikibaseClient->getStore()->getSiteLinkTable(), $wikibaseClient->getLanguageFallbackChainFactory(), $language, $wikibaseClient->getSettings(), $wikibaseClient->getPropertyDataTypeLookup(), + $labelLookup, Utils::getLanguageCodes(), $wikibaseClient->getSettings()->getSetting( 'siteGlobalID' ) ); @@ -58,9 +67,11 @@ */ public function register() { $lib = array( + 'getLabel' => array( $this, 'getLabel' ), 'getEntity' => array( $this, 'getEntity' ), 'getSetting' => array( $this, 'getSetting' ), 'getEntityId' => array( $this, 'getEntityId' ), + 'getSiteLink' => array( $this, 'getSiteLink' ), 'getGlobalSiteId' => array( $this, 'getGlobalSiteId' ) ); @@ -134,4 +145,31 @@ return array( $this->wbLibrary->getSetting( $setting ) ); } + /** + * Wrapper for getLabel in Scribunto_LuaWikibaseLibraryImplementation + * + * @since 0.5 + * + * @param string $prefixedEntityId + * + * @return array + */ + public function getLabel( $prefixedEntityId ) { + $this->checkType( 'getLabel', 1, $prefixedEntityId, 'string' ); + return array( $this->wbLibrary->getLabel( $prefixedEntityId ) ); + } + + /** + * Wrapper for getSiteLink in Scribunto_LuaWikibaseLibraryImplementation + * + * @since 0.5 + * + * @param string $prefixedEntityId + * + * @return array + */ + public function getSiteLink( $prefixedEntityId ) { + $this->checkType( 'getSiteLink', 1, $prefixedEntityId, 'string' ); + return array( $this->wbLibrary->getSiteLink( $prefixedEntityId ) ); + } } diff --git a/client/includes/scribunto/WikibaseLuaBindings.php b/client/includes/scribunto/WikibaseLuaBindings.php index 6fc63e6..f1f5568 100644 --- a/client/includes/scribunto/WikibaseLuaBindings.php +++ b/client/includes/scribunto/WikibaseLuaBindings.php @@ -3,7 +3,10 @@ namespace Wikibase\Client\Scribunto; use Language; +use InvalidArgumentException; +use OutOfBoundsException; use Wikibase\DataModel\Entity\Entity; +use Wikibase\DataModel\Entity\ItemId; use Wikibase\DataModel\Entity\EntityIdParser; use Wikibase\LanguageFallbackChainFactory; use Wikibase\Lib\Serializers\SerializationOptions; @@ -11,7 +14,9 @@ use Wikibase\Lib\Serializers\SerializerFactory; use Wikibase\Lib\Store\EntityLookup; use Wikibase\Lib\Store\SiteLinkLookup; +use Wikibase\Lib\Store\LabelLookup; use Wikibase\DataModel\Entity\PropertyDataTypeLookup; +use Wikibase\DataModel\Entity\EntityIdParsingException; use Wikibase\SettingsArray; /** @@ -71,6 +76,11 @@ private $dataTypeLookup; /** + * @var LabelLookup + */ + private $labelLookup; + + /** * @param EntityIdParser $entityIdParser * @param EntityLookup $entityLookup * @param SiteLinkLookup $siteLinkTable @@ -78,6 +88,7 @@ * @param Language $language * @param SettingsArray $settings * @param PropertyDataTypeLookup $dataTypeLookup + * @param LabelLookup $labelLookup * @param string[] $languageCodes * @param string $siteId */ @@ -89,6 +100,7 @@ Language $language, SettingsArray $settings, PropertyDataTypeLookup $dataTypeLookup, + LabelLookup $labelLookup, $languageCodes, $siteId ) { @@ -101,6 +113,7 @@ $this->languageCodes = $languageCodes; $this->siteId = $siteId; $this->dataTypeLookup = $dataTypeLookup; + $this->labelLookup = $labelLookup; } /** @@ -235,4 +248,46 @@ return $this->settings->getSetting( $setting ); } + /** + * @param string $prefixedEntityId + * + * @since 0.5 + * @return mixed + */ + public function getLabel( $prefixedEntityId ) { + try { + $entityId = $this->entityIdParser->parse( $prefixedEntityId ); + } catch( EntityIdParsingException $e ) { + return ''; + } + + try { + $label = $this->labelLookup->getLabel( $entityId ); + } catch ( OutOfBoundsException $ex ) { + return ''; + } + + return $label; + } + + /** + * @param string $prefixedEntityId + * + * @since 0.5 + * @return string + */ + public function getSiteLink( $prefixedEntityId ) { + try { + $itemId = new ItemId( $prefixedEntityId ); + } catch( InvalidArgumentException $e ) { + return ''; + } + + $item = $this->entityLookup->getEntity( $itemId ); + if ( !$item || !$item->getSiteLinkList()->hasLinkWithSiteId( $this->siteId ) ) { + return ''; + } + + return $item->getSiteLinkList()->getBySiteId( $this->siteId )->getPageName(); + } } diff --git a/client/includes/scribunto/mw.wikibase.lua b/client/includes/scribunto/mw.wikibase.lua index db45d52..a1802ec 100644 --- a/client/includes/scribunto/mw.wikibase.lua +++ b/client/includes/scribunto/mw.wikibase.lua @@ -63,7 +63,7 @@ -- Get the mw.wikibase.entity object for the current page wikibase.getEntityObject = function( id ) if id ~= nil and type( id ) ~= 'string' then - error( 'Id must be either of type string or nil, ' .. type( id ) .. ' given' ) + error( 'id must be either of type string or nil, ' .. type( id ) .. ' given', 2 ) end if id == nil then @@ -71,7 +71,7 @@ end if not php.getSetting( 'allowArbitraryDataAccess' ) and id ~= getEntityIdForCurrentPage() then - error( 'Access to arbitrary items has been disabled.' ) + error( 'Access to arbitrary items has been disabled.', 2 ) end if id == nil then @@ -85,26 +85,22 @@ -- -- @param id wikibase.label = function( id ) - local entity = getEntityObject( id ) - - if entity == nil then - return nil + if type( id ) ~= 'string' then + error( 'id must be of type string, ' .. type( id ) .. ' given', 2 ) end - return entity:getLabel() + return php.getLabel( id ) end -- Get the local sitelink title for the given entity id (if one exists) -- -- @param id wikibase.sitelink = function( id ) - local entity = getEntityObject( id ) - - if entity == nil then - return nil + if type( id ) ~= 'string' then + error( 'id must be of type string, ' .. type( id ) .. ' given', 2 ) end - return entity:getSitelink() + return php.getSiteLink( id ) end mw = mw or {} diff --git a/client/tests/phpunit/includes/scribunto/LuaWikibaseLibraryTests.lua b/client/tests/phpunit/includes/scribunto/LuaWikibaseLibraryTests.lua index 2b5cf80..2130484 100644 --- a/client/tests/phpunit/includes/scribunto/LuaWikibaseLibraryTests.lua +++ b/client/tests/phpunit/includes/scribunto/LuaWikibaseLibraryTests.lua @@ -53,7 +53,7 @@ }, { name = 'mw.wikibase.getEntityObject (id must be string)', func = mw.wikibase.getEntityObject, args = { 123 }, - expect = 'Id must be either of type string or nil, number given' + expect = 'id must be either of type string or nil, number given' }, { name = 'mw.wikibase.label', func = mw.wikibase.label, type='ToString', args = { 'Q32487' }, diff --git a/client/tests/phpunit/includes/scribunto/WikibaseLuaBindingsTest.php b/client/tests/phpunit/includes/scribunto/WikibaseLuaBindingsTest.php index 236fe9d..863e5c9 100644 --- a/client/tests/phpunit/includes/scribunto/WikibaseLuaBindingsTest.php +++ b/client/tests/phpunit/includes/scribunto/WikibaseLuaBindingsTest.php @@ -54,6 +54,11 @@ ->method( 'getDataTypeIdForProperty' ) ->will( $this->returnValue( 'structured-cat' ) ); + $labelLookup = $this->getMock( 'Wikibase\Lib\Store\LabelLookup' ); + $labelLookup->expects( $this->any() ) + ->method( 'getLabel' ) + ->will( $this->returnValue( 'LabelString' ) ); + return new WikibaseLuaBindings( new BasicEntityIdParser(), $entityLookup ? $entityLookup : new MockRepository(), @@ -62,6 +67,7 @@ $language, // language new SettingsArray(), $propertyDataTypeLookup, + $labelLookup, array( 'de', 'en', 'es', 'ja' ), "enwiki" // siteId ); @@ -107,6 +113,47 @@ $this->assertEquals( 'enwiki', $wikibaseLibrary->getGlobalSiteId() ); } + public function getLabelProvider() { + return array( + array( 'LabelString', 'Q123' ), + array( '', 'DoesntExist' ) + ); + } + + /** + * @dataProvider getLabelProvider + * + * @param string $expected + * @param string $itemId + */ + public function testGetLabel( $expected, $itemId ) { + $wikibaseLibrary = $this->getWikibaseLibraryImplementation(); + $this->assertEquals( $expected, $wikibaseLibrary->getLabel( $itemId ) ); + } + + public function getSiteLinkProvider() { + return array( + array( 'Beer', 'Q666' ), + array( '', 'DoesntExist' ) + ); + } + + /** + * @dataProvider getSiteLinkProvider + * + * @param string $expected + * @param string $itemId + */ + public function testGetSiteLink( $expected, $itemId ) { + $item = $this->getItem(); + + $entityLookup = new MockRepository(); + $entityLookup->putEntity( $item ); + + $wikibaseLibrary = $this->getWikibaseLibraryImplementation( $entityLookup ); + $this->assertSame( $expected, $wikibaseLibrary->getSiteLink( $itemId ) ); + } + protected function getItem() { $itemId = new ItemId( 'Q666' ); -- To view, visit https://gerrit.wikimedia.org/r/171992 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic96dca7dc7abce339b1ce05d678db001fd432189 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Hoo man <h...@online.de> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits