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

Reply via email to